| Summary: | Fail:check-print-options | ||
|---|---|---|---|
| Product: | pkg-config | Reporter: | Daniel Macks <dmacks> |
| Component: | src | Assignee: | Dan Nicholson <dbn.lists> |
| Status: | RESOLVED MOVED | QA Contact: | |
| Severity: | normal | ||
| Priority: | medium | ||
| Version: | unspecified | ||
| Hardware: | Other | ||
| OS: | Mac OS X (All) | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: | Sort output of --print-variables | ||
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? 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. 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. 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. The patch I proposed appears to have been committed as 818aeace3b08d731d104d5ef9b1d5e3c34dd4192 (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. -- 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.
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).