Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

maximum_package_count is off by one when limiting number of packages to consider #295

Closed
jashank opened this issue Apr 27, 2023 · 4 comments

Comments

@jashank
Copy link

jashank commented Apr 27, 2023

Hello!

Scenario: I have packages atf-c, atf-c++, atf-sh. (Any set of two or more packages works for this trick; I happen to have these three from the same source alphabetically early in my list of packages.)

$ pkgconf --modversion atf-c atf-c++ atf-sh
0.20
0.20

... so where did the third one go?

Spelunking the code, I see that --modversion (and some other flags) put us into a mode where we only consider one package at a time by setting maximum_package_count — perfectly reasonable (though I don't know if this behaviour is actually documented anywhere except here in the source, so surprising in that regard):

pkgconf/cli/main.c

Lines 1113 to 1127 in 78f3abc

/* if these selectors are used, it means that we are inquiring about a single package.
* so signal to libpkgconf that we do not want to use the dependency resolver for more than one level,
* and also limit the SAT problem to a single package.
*/
if (((want_flags & PKG_REQUIRES) == PKG_REQUIRES ||
(want_flags & PKG_REQUIRES_PRIVATE) == PKG_REQUIRES_PRIVATE ||
(want_flags & PKG_PROVIDES) == PKG_PROVIDES ||
(want_flags & PKG_VARIABLES) == PKG_VARIABLES ||
(want_flags & PKG_MODVERSION) == PKG_MODVERSION ||
(want_flags & PKG_PATH) == PKG_PATH ||
want_variable != NULL))
{
maximum_package_count = 1;
maximum_traverse_depth = 1;
}

However, the logic to actually enforce this erroneously compares cardinal pkgq.length to ordinal maximum_package_count:

pkgconf/cli/main.c

Lines 1324 to 1328 in 78f3abc

/* check if there is a limit to the number of packages allowed to be included, if so and we have hit
* the limit, stop adding packages to the queue.
*/
if (maximum_package_count > 0 && pkgq.length > maximum_package_count)
break;

(I stumbled across this bug because Cabal does this wrong then fails for mysterious reasons; I'll also file a bug there about this behaviour.)

@kaniini
Copy link
Member

kaniini commented May 2, 2023

Fixed in pending 1.9.5 release. Thanks!

@kaniini kaniini closed this as completed May 2, 2023
@Dretch
Copy link

Dretch commented May 6, 2023

I wonder if you would consider making pkgconf fail (with a non-zero exit code) when given multiple packages (e.g. pkgconf --modversion atf-c atf-c++ atf-sh).

Some motivation for this:

@juhp
Copy link

juhp commented Jul 7, 2023

I also stumbled across this today (initially with 1.9.4) and also in the context of Haskell cabal.
But I don't think it was "off-by-one": 1.9.4 never printed more than two versions for me!

And 1.9.5 only ever seems to print one version for me:

$ pkgconf --version
1.9.5
$ pkgconf --modversion pango glib-2.0 gio-2.0 harfbuzz 
1.50.14
$ 

Anyway I can open a new issue, since this is closed, though not fixed afaict - re-opening would also make sense.

@juhp
Copy link

juhp commented Jul 7, 2023

Ah wait you are saying that outputting one package version now is the correct behavior I see...
Then haskell Cabal needs to be fixed...

And +1 for erroring for more than one package

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants