Opened 5 years ago
Closed 5 years ago
#59538 closed defect (fixed)
github portgroup doesn't work with default master_sites from other portgroups
Reported by: | jmroot (Joshua Root) | Owned by: | |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | ports | Version: | |
Keywords: | Cc: | Schamschula (Marius Schamschula), reneeotten (Renee Otten), ryandesign (Ryan Carsten Schmidt) | |
Port: |
Description
The lack of renaming in #59525 seems to be because this check was false: https://github.com/macports/macports-ports/blob/master/_resources/port1.0/group/github-1.0.tcl#L92
This is because the github portgroup only sets a default for master_sites; github.setup doesn't set it to a definite value. Is it intended that you should be able to use different master_sites after calling github.setup? If so, how is that supposed to behave? If not, I imagine github.setup should just set master_sites and the default is unnecessary.
Change History (12)
comment:1 Changed 5 years ago by ryandesign (Ryan Carsten Schmidt)
comment:2 Changed 5 years ago by jmroot (Joshua Root)
Well we apparently need to have some kind of specification for how the proc behaves. Does it set master_sites as needed to download from github, or append to it, or do nothing to it?
comment:3 Changed 5 years ago by ryandesign (Ryan Carsten Schmidt)
It sets a default for master_sites that is appropriate for downloading the type of GitHub file specified in github.tarball_from. Ports can and do override master_sites. The portgroup used to set master_sites, but it was changed 8.5 years ago to only set a default because that made some things easier.
The portgroup is kind of terrible at this point. I tried maybe a year ago to spec out a new github-2.0 portgroup to fix all of its problems, including wanting to be able to download any number of files from any number of different GitHub projects (see for example the MoltenVK port), but I didn't reach a satisfactory conclusion; I couldn't decide how everything should work.
We should add support to MacPorts base for downloading from different types of GitHub files, just as base already has support for downloading from SourceForge. The new portgroup could then use that.
comment:4 Changed 5 years ago by jmroot (Joshua Root)
Unfortunately we need a fix for right now too. If github.setup set a default, that would work, but it doesn't. My expectation would be that github.setup changes master_sites so that the distfiles will be downloaded from github. If it used default, that would solve the problem from #33826 too. (Though I note that if you set github.tarball_from before changing git.branch, you would still see that problem to this day.)
comment:5 Changed 5 years ago by ryandesign (Ryan Carsten Schmidt)
The github portgroup sets a default for master_sites when it is included. Why would setting that default again (or instead) when github.setup is called be helpful?
comment:6 Changed 5 years ago by jmroot (Joshua Root)
The issue is occurring when the python portgroup is included after the github one. The new default overwrites the previous one.
comment:7 Changed 5 years ago by ryandesign (Ryan Carsten Schmidt)
Alright. It sounds like your suggestion makes sense then:
Index: github-1.0.tcl =================================================================== --- github-1.0.tcl (revision 164360) +++ github-1.0.tcl (working copy) @@ -20,8 +20,6 @@ options github.master_sites default github.master_sites {${github.homepage}/tarball/${git.branch}} -default master_sites {${github.master_sites}} - options github.tarball_from default github.tarball_from tarball option_proc github.tarball_from handle_tarball_from @@ -77,6 +75,7 @@ default homepage ${github.homepage} git.url ${github.homepage}.git git.branch [join ${github.tag_prefix}]${github.version}[join ${github.tag_suffix}] + default master_sites {${github.master_sites}} distname ${github.project}-${github.version} post-extract {
But I haven't tested this.
comment:8 Changed 5 years ago by reneeotten (Renee Otten)
this change solves the issue I was seeing yesterday when using both the github and python PG; but I cannot reaaly tell if there are unintended side-effects from this change. If not, I'd vote for making this change.
comment:9 Changed 5 years ago by ryandesign (Ryan Carsten Schmidt)
I can commit it tonight, if nobody else beats me to it. But if you do commit it, please monitor the buildbot for failures and see if they might be because of this change.
comment:10 Changed 5 years ago by jmroot (Joshua Root)
The funny thing is, the documentation for the github portgroup is silent on whether it sets master_sites. The reader could infer that it does (because how else is something like github.tarball_from useful), but it's never actually stated. While Hyrum's Law is going to apply regardless, specifying more fully what portgroups are and aren't going to do for you might help.
comment:11 Changed 5 years ago by reneeotten (Renee Otten)
comment:12 Changed 5 years ago by ryandesign (Ryan Carsten Schmidt)
Resolution: | → fixed |
---|---|
Status: | new → closed |
So I guess that fixed it.
A zillion ports override the github portgroups' master_sites. I haven't audited them to see how many of them rely on the fact that they can do so before calling github.setup. The intended use was that github.setup would be called near the top of the portfile, in place of the name and version lines, but many many ports adopted a different style in which github.setup is called in several different subports.