Bug 79666 - Fail:check-print-options
Summary: Fail:check-print-options
Status: RESOLVED MOVED
Alias: None
Product: pkg-config
Classification: Unclassified
Component: src (show other bugs)
Version: unspecified
Hardware: Other Mac OS X (All)
: medium normal
Assignee: Dan Nicholson
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-06-05 05:49 UTC by Daniel Macks
Modified: 2018-08-25 12:56 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Sort output of --print-variables (1.57 KB, patch)
2014-06-05 07:08 UTC, Daniel Macks
Details | Splinter Review

Description Daniel Macks 2014-06-05 05:49:02 UTC
Building pkg-config-0.28 on OS X 10.8 against an already-installed glib-2.22.4 succeeds with no errors or warnings. It fails a self-test:

../pkg-config --print-variables simple :
'prefix
libdir
exec_prefix
includedir' != 'exec_prefix
prefix
libdir
includedir'
FAIL: check-print-options

It looks like the same set of variable-names, just in a different order. Neither order appears to have any significance. Neither the expected nor actual result is n the order they appear in the simple.pc test file (which doesn't seem a useful detail to retain anyway) nor sorted alphabetically (which would be a nicely deterministic effect). If I understand the code being tested, it's main.c around line 685:

  g_hash_table_foreach(pkg->vars,
                       &print_hashtable_key,
                       NULL);

and it's failing because it's assuming that a hash will have a particular order. I can't find that documented as such in the glib docs, and it goes against the whole idea of a hash (vs a list) data structure.

One solution would be to sort the list of keys. Something like (untested):

  GList *varnames = g_hash_table_get_values(pkg->vars);
  varnames = g_list_sort(varnames, (GCompareFunc)g_ascii_strcasecmp);
  g_list_foreach(sorted_varnames,
                 &print_list_entry,
                 NULL);

with print_list_entry being a printf() of its first argument (like print_hashtable_key).
Comment 1 Daniel Macks 2014-06-05 07:08:54 UTC
Created attachment 100443 [details] [review]
Sort output of --print-variables

...which is exactly why I shouldn't write code in bugzilla. Attached patch actually uses consistent variable-names and correct parts of the hash. Compiles with no warnings; only uses g_* functions that have existed since at least as far back as 2.16 (target minimum per NEWS entry for 0.27, though I only have 2.22.4 to actually test) and that are also present in the onboard glib/ chunks. With this patch, I get PASS:check-print-options

I'm not sure g_ascii_strcasecmp() is the best choice here. Do we care about case-sensitivity? Do we need to handle non-ascii chars?
Comment 2 Dan Nicholson 2014-09-27 22:53:01 UTC
Sorry for the super slow reply. If the point is to make things deterministic, wouldn't it be better to use strcmp? With strcasecmp, two variables that only differ in case would again be subject to the ordering in the hashing.
Comment 3 Daniel Macks 2014-09-28 08:27:36 UTC
Case-sensitive would certainly be reasonable.

Now thinking about it, one of the platforms where pkg-config is a standard tool is OS X, which has by default a case-insensitive filesystem. I'm not sure about other "non-linux"-derived platforms. That means we can't reliably support having two .pc that differ only by case at all even though pkg-config is case-respecting for the filenames--one would overwrite the other even if they are intended to be "different" libraries. Sounds like fodder for another bug.
Comment 4 Dan Nicholson 2014-11-12 15:33:21 UTC
Windows has the same problem with case insensitive filesystem, but that's not a issue pkg-config has to worry about. On those systems, there can't be two .pc files that differ only in case and the case sensitive string comparison won't matter because it will never encounter that scenario. But on systems with case sensitive filesystems, the string comparison will matter and will allow the ordering to be deterministic.

I don't think we should spend time worrying about case insensitive filesystems here. We can't do anything about that in pkg-config.
Comment 5 Daniel Macks 2017-12-16 07:21:26 UTC
The patch I proposed appears to have been committed as 	818aeace3b08d731d104d5ef9b1d5e3c34dd4192
Comment 6 Daniel Macks 2017-12-16 07:48:25 UTC
(In reply to Daniel Macks from comment #5)
> The patch I proposed appears to have been committed as 
> 818aeace3b08d731d104d5ef9b1d5e3c34dd4192

...with your improved idea to use g_strcmp0 for deterministic case-sensitivity.
Comment 7 GitLab Migration User 2018-08-25 12:56:59 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/pkg-config/pkg-config/issues/48.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.