GNOME Bugzilla – Bug 748332
wrong module sets included
Last modified: 2016-03-31 15:55:58 UTC
There is bug including module set if uri has query part. For example: https://git.gnome.org/browse/gnome-modulesets/plain/gnome-world.modules?h=gnome-3-16 jhbuild will include gnome-world.modules?h=gnome-3-16. Next it will try to include gnome-apps.modules and it will include wrong one. Building new uri to include it will be: https://git.gnome.org/browse/gnome-modulesets/plain/gnome-apps.modules it is generated/created without query part. I think it is python bug, but I will attach patch for jhbuild that will fix this bug.
Created attachment 302182 [details] [review] fix module set including with uri that contains query part
There's no reference to the gnome-modulesets git module in jhbuild, there's a jjardon/gnome-modulesets branch that uses this location but it's temporary, as discussed in bug 675873. I prefer to avoid that change for now; if modulesets hosted behing URIs with query strings become popular, feel free to reopen the bug and I'll look at the patch. (I would have told you before the patch)
In jhbuild only one version of gnome-world is available and it includes current development version gnome-apps.modules. That means there is no way to build 3.16 including gnome-world.modules in custom module set file. I think that urlparse.urljoin should keep base url query part if relative URL does not have. From python doc: "Construct a full (“absolute”) URL by combining a “base URL” (base) with another URL (url). Informally, this uses components of the base URL, in particular the addressing scheme, the network location and (part of) the path, to provide missing components in the relative URL." I am proposing simply manually adding query to include uri if it does not have query part. It is only 4 extra lines. Also if/when python bug will be fixed it should not break anything.
Sorry I didn't see the relation between the topic of this bug report and the fact that gnome-world.modules is tied to the development version of GNOME. So, you want to create a custom moduleset but you want it to use gnome-world but you want an older version of gnome-world, so you try to add an <include href="..."/> pointing to an arbitrary URI with a query string and it doesn't work. It works for me, for example I tried to include the moduleset just before it was switched to include 3.18: <?xml version="1.0"?> <moduleset> <include href="https://git.gnome.org/browse/jhbuild/plain/modulesets/gnome-world.modules?id=5eb44be7ebd41dc54e444592f8df1268b01cdcb4"/> </moduleset> And didn't get any error. Now I think I get it, you'd want the <include href="gnome-apps.modules"/> that is present in https://git.gnome.org/browse/gnome-modulesets/plain/gnome-world.modules?h=gnome-3-16 to refer to https://.../gnome-apps.modules?h=gnome-3-16. I believe you want a really special behaviour, and I wouldn't even consider the current behaviour as a bug (click on a href="foobar" link on a page with a "?blah" query string, you get yourself on a "foobar" URI, not a "foobar?blah" URI). I'd suggest you to ignore the gnome-moduleset repository, there's a reason why I asked the sysadmins for a proper layout in bug 675934. In the meantime you can point to the URI in the jhbuild repository, like I did above.
Well using that URI is not solution to me - it will not be possible to build 3.16 since libgweather at that moment was using master branch. gnome-modulesets repository has same problem - 3.16 is not using gnome-3-16 branch. You are right with your example, but jhbuild is not web. I opened bug for python, lets see what they will say. Maybe I am wrong, but right now I still think that urljoin should add query string.
Created attachment 312701 [details] [review] config: add include_keep_query config option There might be valid cases when we want to pass query part to all include uris. For example: git.gnome.org/browse/gnome-modulesets/plain/gnome-world.modules?h=gnome-3-18 In this case we want to include 'gnome-apps.modules?h=gnome-3-18' not 'gnome-apps.modules' which is default behavior. Frederic, what do you thing about this patch?
I still stand by comment 4. The proper solution is to your usecase is to get bug 675934 fixed.
(In reply to Frederic Peters from comment #7) > I still stand by comment 4. > > The proper solution is to your usecase is to get bug 675934 fixed. That bug is open from 2012-05-12... :( I agree that it would fix my usecase, but that does not mean that in future there might not be other cases when similar option will be needed. Is there something wrong with having option to change default behavior? Or maybe you have some info when that bug might get fixed?