Ensure that provides/requires versions are factored into dep filtering #68
Conversation
| if (g_hash_table_lookup_extended(provided_hashtable, filename, NULL, &pvalue)) { | ||
| struct ap_value_struct *ap_value = pvalue; | ||
| if (!ap_value->flags || !ap_value->flags[0] || !flags || !flags[0] || | ||
| (!g_strcmp0(ap_value->flags, flags) && !strcmp(ap_value->version, full_version))) |
There was a problem hiding this comment.
Addressed in glibify commit
| GHashTable *provided_hashtable = g_hash_table_new_full(g_str_hash, | ||
| g_str_equal, | ||
| NULL, | ||
| free); |
There was a problem hiding this comment.
Addressed in glibify commit
There was a problem hiding this comment.
(you're aware that all those non-glib code is just copy'n'paste from some lines above/below? Now doing some glibification makes the code pretty inconsistent...)
There was a problem hiding this comment.
@mlschroe then someone have to change code from where it was copied ;)
| pkg->provides = g_slist_prepend(pkg->provides, dependency); | ||
| struct ap_value_struct *pvalue = malloc(sizeof(struct ap_value_struct)); |
There was a problem hiding this comment.
Addressed in glibify commit
Conan-Kudo
force-pushed
the
fixdepfiltering
branch
from
8ada5d4
to
d0de224
Compare
|
@Conan-Kudo also would be nice to get test for this. |
Conan-Kudo
force-pushed
the
fixdepfiltering
branch
from
d0de224
to
ae1d25f
Compare
|
@ignatenkobrain I'm not sure how to write tests for this. And in any case, this fix is rather important, since at least in Mageia, this broke the GNOME stack. It can break things subtly in other places using createrepo_c, like COPR and eventually Koji. |
|
@Tojaj What do you think of the fix? |
| gpointer pvalue; | ||
| if (g_hash_table_lookup_extended(provided_hashtable, filename, NULL, &pvalue)) { | ||
| struct ap_value_struct *ap_value = pvalue; | ||
| if (!ap_value->flags || !ap_value->flags[0] || !flags || !flags[0] || |
There was a problem hiding this comment.
If I understand this correctly then:
I we already have a file which doesn't have any flags in the hash table (!ap_value->flags) then we don't care about any other files with the same filename even if flags and versions are different?
| struct ap_value_struct *ap_value = pvalue; | ||
| if (!ap_value->flags || !ap_value->flags[0] || !flags || !flags[0] || | ||
| (!g_strcmp0(ap_value->flags, flags) && !g_strcmp0(ap_value->version, full_version))) | ||
| continue; |
There was a problem hiding this comment.
As the condition is on two lines and indentation could be confusing, could you please put curly brackets around the continue; single line scope?
This should avoid situations where a package provides and requires same capability (a dependency) with different versions but createrepo_c strip the require down (because it already saw it in provides). This should fix the issue in a better way than PR #68
|
I created a new PR #70 which uses name-flags-version for dep checking, thus with this patch, if a package provides |
|
Closing in favor of #70 |
This PR includes a patch from @mlschroe that fixes #67.
Reference: https://bugs.mageia.org/show_bug.cgi?id=19509