~skeeto/public-inbox

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
6 2

[PATCH] Fix failing due to non-existent private package

Details
Message ID
<cyzr422npxkldaoirrrelwsronyetppusxewb4y4yo5p7olnky@zwqe36nsd4tp>
Sender timestamp
1741531706
DKIM signature
pass
Download raw message
Patch: +55 -7
---
 test_main.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 u-config.c  | 16 +++++++++-------
 2 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/test_main.c b/test_main.c
index bf19442..a476ef9 100644
--- a/test_main.c
+++ b/test_main.c
@@ -414,6 +414,51 @@ static void test_private_transitive(void)
    EXPECT("-la -lx -lb -lc\n");
}

static void test_private_non_existing(void)
{
    // Scenario: a privately requires non-existing b
    // Expect: --libs a should not fail unless --static is also used
    config conf = newtest_(S("private non existing package"));
    newfile_(&conf, S("/usr/lib/pkgconfig/a.pc"), S(
        PCHDR
        "Requires.private: b\n"
        "Libs: -la\n"
    ));
    // Same situation but with a level of indirection
    newfile_(&conf, S("/usr/lib/pkgconfig/c.pc"), S(
        PCHDR
        "Requires: d\n"
        "Libs: -lc\n"
    ));
    newfile_(&conf, S("/usr/lib/pkgconfig/d.pc"), S(
        PCHDR
        "Requires.private: e\n"
        "Libs: -ld\n"
    ));

    SHOULDPASS {
        run(conf, S("--libs"), S("a"), E);
    }
    EXPECT("-la\n");

    SHOULDFAIL {
        run(conf, S("--libs"), S("--static"), S("a"), E);
    }
    MATCH("not find");
    MATCH("b");

    SHOULDPASS {
        run(conf, S("--libs"), S("c"), E);
    }
    EXPECT("-lc -ld\n");

    SHOULDFAIL {
        run(conf, S("--static"), S("--libs"), S("c"), E);
    }
    MATCH("not find");
    MATCH("e");
}

static void test_revealed_transitive(void)
{
    // Scenario: a privately requires b, which requires x
@@ -833,6 +878,7 @@ int main(void)
    test_overrides();
    test_maximum_traverse_depth();
    test_private_transitive();
    test_private_non_existing();
    test_revealed_transitive();
    test_syspaths();
    test_libsorder();
diff --git a/u-config.c b/u-config.c
index cccf94e..8013aaf 100644
--- a/u-config.c
+++ b/u-config.c
@@ -1460,7 +1460,7 @@ static void failversion(u8buf *err, pkg *pkg, versop op, s8 want)
    os_fail();
}

static pkgs process(processor *proc, pkgspec *specs, arena *perm)
static pkgs process(processor *proc, pkgspec *specs, int static_, arena *perm)
{
    u8buf *err = proc->err;
    pkgs pkgs = {0};
@@ -1527,7 +1527,7 @@ static pkgs process(processor *proc, pkgspec *specs, arena *perm)
            }

            if (proc->recursive && depth<proc->maxdepth) {
                if (top >= cap-2) {
                if (top >= cap-(1+static_)) {
                    failmaxrecurse(err, p->name);
                }
                top++;
@@ -1535,10 +1535,12 @@ static pkgs process(processor *proc, pkgspec *specs, arena *perm)
                stack[top].depth = depth;
                stack[top].flags = flags & ~pkg_DIRECT;

                top++;
                stack[top].specs = p->specs_requiresprivate;
                stack[top].depth = depth;
                stack[top].flags = 0;
                if (static_) {
                    top++;
                    stack[top].specs = p->specs_requiresprivate;
                    stack[top].depth = depth;
                    stack[top].flags = 0;
                }
            }
        }
        p->flags |= flags;
@@ -2010,7 +2012,7 @@ static void uconfig(config *conf)
    }

    pkgspec *specs = parsespecs(args, nargs, 0, err, perm);
    pkgs pkgs = process(proc, specs, perm);
    pkgs pkgs = process(proc, specs, static_, perm);

    if (!pkgs.count) {
        prints8(err, S("pkg-config: "));
-- 
2.48.1
Details
Message ID
<20250309153934.qeti2obbwokmlxbb@nullprogram.com>
In-Reply-To
<cyzr422npxkldaoirrrelwsronyetppusxewb4y4yo5p7olnky@zwqe36nsd4tp> (view parent)
Sender timestamp
1741520374
DKIM signature
missing
Download raw message
Thanks, NRK! At first I thought this might have been a regression from 
recent changes, but soon realized it's not. So what prompted this now? A 
real world situation?

I merged it, but made a couple of minor tweaks:

* Moved static_ onto processor. That's where similar options live. It's a 
little awkward using it outside a "processor" context at the bottom, but 
oh well. (Now that it's no longer concerned with parsing, perhaps I should 
rename it to "loader" because it loads packages.)

* Wrapped package names in single quotes in MATCH. Otherwise "e" trivially 
matches 'e' in "package" in the error message. In the new error message 
tests I deliberately chose names that would never appear as substrings 
elsewhere. Perhaps MATCH should require that the start and end of matches 
land on word boundaries, too, like implicit regex angle brackets.
Details
Message ID
<krmpupuaclniq7bpprqsu2mr4j6uhm2fa5xhprmuk3jakcfwnu@2qbm2qbgbarr>
In-Reply-To
<20250309153934.qeti2obbwokmlxbb@nullprogram.com> (view parent)
Sender timestamp
1741535582
DKIM signature
pass
Download raw message
On Sun, Mar 09, 2025 at 11:39:34AM -0400, Christopher Wellons wrote:
> Thanks, NRK! At first I thought this might have been a regression from
> recent changes, but soon realized it's not. So what prompted this now? A
> real world situation?

Yup, I noticed last night that libplacebo pc file has private
requirement on vulkan (even though I built it without vulkan support;
upstream bug?? need to investigate) and I recently removed vulkan from
my system. And so it started failing even though `--static` wasn't used.

> * Moved static_ onto processor. That's where similar options live.

I had considered that too. I think it's more natural.

> * Wrapped package names in single quotes in MATCH. Otherwise "e" trivially
> matches 'e' in "package" in the error message.

Ah, right. Good catch. I wrote the patch in a hurry last night and so
wasn't thinking clearly.

- NRK
Details
Message ID
<tjk7d3bx6tiv7mukjbla5v43ykhptwfvdnjipfj274pwfdqw5s@pdd2omofjpum>
In-Reply-To
<krmpupuaclniq7bpprqsu2mr4j6uhm2fa5xhprmuk3jakcfwnu@2qbm2qbgbarr> (view parent)
Sender timestamp
1741562143
DKIM signature
pass
Download raw message
> Yup, I noticed last night that libplacebo pc file has private
> requirement on vulkan (even though I built it without vulkan support;
> upstream bug?? need to investigate) and I recently removed vulkan from
> my system.

Hmm, this is no longer an issue after re-installing libplacebo. Weird.
Somehow I ended up with a stale pc file? If I had disabled vulkan
support in my package configuration but hadn't yet rebuilt libplacebo
then portage would've nagged me to rebuild and it also wouldn't have
allowed me to uninstall vulkan to begin with.

So yeah, I'm not sure how that happened. But I guess it worked out for
the best as it allowed me to sniff out a u-config bug all things
considered.

- NRK
Details
Message ID
<20250310010028.7afs2k6k4j5kj5py@nullprogram.com>
In-Reply-To
<tjk7d3bx6tiv7mukjbla5v43ykhptwfvdnjipfj274pwfdqw5s@pdd2omofjpum> (view parent)
Sender timestamp
1741554028
DKIM signature
missing
Download raw message
Thanks for the background info.

> sniff out a u-config bug all things considered

Was this actually a bug, though? I just checked, and neither pkgconf nor 
Freedesktop.org pkg-config ignore missing Requires.private packages. The 
new behavior is more like a feature, allowing builds to go forward even 
when unneeded information is missing. As for which is most correct, old or 
new behavior, I think it could be reasonably argued either way.
Details
Message ID
<lmi7mnoqp677srwj7he6fbumhbjlfnsbxaaszztefdsmlc22by@42555asxtqvx>
In-Reply-To
<20250310010028.7afs2k6k4j5kj5py@nullprogram.com> (view parent)
Sender timestamp
1741610681
DKIM signature
pass
Download raw message
> Was this actually a bug, though? I just checked, and neither pkgconf nor
> Freedesktop.org pkg-config ignore missing Requires.private packages.

I haven't checked freedesktop one, but lastest pkgconf available in
gentoo (2.3.0) does in fact ignore missing private requires.

	~> cat test.pc
	prefix=/usr
	includedir=${prefix}/include
	libdir=${prefix}/lib64
	
	Name: test
	Description: test
	Version: 0.0.0
	Libs: -L${libdir} -ltest
	Cflags: -I${includedir}
	Requires: vulkan
	~> pkgconf --libs ./test.pc
	Package vulkan was not found in the pkg-config search path.
	Perhaps you should add the directory containing `vulkan.pc'
	to the PKG_CONFIG_PATH environment variable
	Package 'vulkan', required by 'test', not found
	~> sed -i 's|Requires|Requires.private|' test.pc
	~> pkgconf --libs ./test.pc
	-ltest
	~> ./pkgconf/bin/pkgconf --libs --static ./test.pc
	Package vulkan was not found in the pkg-config search path.
	Perhaps you should add the directory containing `vulkan.pc'
	to the PKG_CONFIG_PATH environment variable
	Package 'vulkan', required by 'test', not found
	~> pkgconf --version
	2.3.0

Perhaps your pkgconf version is older and this is a new fix/feature?
I presumed it was a bug since (a) it doesn't make sense to search for
private requirements without --static and (b) the pkgconf available at
my system also didn't fail on it without --static.

- NRK
Details
Message ID
<20250310155732.my7gsrtwmsqvopoh@nullprogram.com>
In-Reply-To
<lmi7mnoqp677srwj7he6fbumhbjlfnsbxaaszztefdsmlc22by@42555asxtqvx> (view parent)
Sender timestamp
1741607852
DKIM signature
missing
Download raw message
I was testing straight from the repository, not my system's pkg-config. I 
dug a little further, and it turns out it's not because my pkgconf is too 
old but too new. As of pkgconf 2.4.0, specifically a79952a (2025-02-02), 
your example produces an error. So they went the opposite direction from 
your change.

(Unfortunately it's difficult to test directly due to botched fgetline 
changes in d0f8f3f which introduced a buffer overflow. All commits from 
that until 88258bd crash for me. 88258bd doesn't fix the overflow, which 
as far as I can tell, is still present as of pkgconf 2.4.3. It merely 
reduced occurrence to specific large inputs rather than any input.)
Reply to thread Export thread (mbox)