Opened 7 years ago

Last modified 2 years ago

#56369 new defect

github portgroup: don't override an already-specified version

Reported by: ryandesign (Ryan Carsten Schmidt) Owned by:
Priority: Normal Milestone:
Component: ports Version:
Keywords: portgroup Cc: pmetzger (Perry E. Metzger)
Port:

Description

Perry recently encountered the need to write the following in a Portfile:

set                 _version 1.13
github.setup        xavierleroy cryptokit [string map {. ""} $_version] release
version             $_version

That is, the developers created the version number in the tag name lossily from the real version (the period was removed). Perry wanted to specify the real version, and compute the lossy tag version from that. But github.setup unconditionally sets version from that. Perry wanted that to be changed, so that, much as it already only sets name if the name has not already been set, it should only set version if the version has not already been set.

It would have to be handled in the portgroup the same unusual way that setting name is handled, using PortInfo, because doing it the expected way, using default, does not work correctly, for reasons I have not fully understood. I will attach a patch of that as an example, but it should not be committed yet; see below.

The more important problem is that, although the github portgroup was designed with the intention that github.setup would be invoked exactly once in any portfile, portfile authors have taken to running it once in each subport instead. And specifically, there are cases where it is called once unconditionally at the top of the portfile to set the main port's version, and then called a second time in a -devel subport to override the version for the subport. An example of such a port is cantera. If we were to implement Perry's suggestion, this usage would break, and those subports would then be at the wrong version.

We either cannot implement the suggestion, or we need to identify all of the portfiles where github.setup will be called more than once and change them so that they no longer do that. After that is done, we could also modify the github portgroup to print an error if github.setup is called a second time.

Whatever we do for the github portgroup should also be done for the bitbucket portgroup.

Attachments (1)

github-1.0-version.diff (545 bytes) - added by ryandesign (Ryan Carsten Schmidt) 7 years ago.

Download all attachments as: .zip

Change History (6)

Changed 7 years ago by ryandesign (Ryan Carsten Schmidt)

Attachment: github-1.0-version.diff added

comment:1 Changed 7 years ago by ryandesign (Ryan Carsten Schmidt)

The longer-term project is to remove the github.setup procedure entirely, because we now consider .setup procedures to be an antipattern. Once we do that, the problem goes away. But I haven't given sufficient thought to that project yet to be able to implement it.

comment:2 in reply to:  1 Changed 7 years ago by raimue (Rainer Müller)

Replying to ryandesign:

The longer-term project is to remove the github.setup procedure entirely, because we now consider .setup procedures to be an antipattern.

We do not want .setup? Any more information on that? Almost every port group uses this style.

comment:3 Changed 7 years ago by ryandesign (Ryan Carsten Schmidt)

Many do. Many don't. I'm pretty sure we said many years ago that .setup procedures are bad, and that it's better to just specify things directly, which is why the python and php portgroups were then designed that way, without .setup procedures.

comment:4 Changed 4 years ago by ryandesign (Ryan Carsten Schmidt)

One reason why .setup procedures are bad is that things become thoroughly confusing if a port needs to include two or more portgroups each of which has a setup procedure. See for example browser:macports-ports/multimedia/xmltv/Portfile. Setup procedures typically like to set the port name and version for you based on the parameters you specify. If you need to include more than one setup procedure, which one sets those variables for you? Which one do you call first? Does it matter? Sometimes it does.

comment:5 Changed 2 years ago by mascguy (Christopher Nielsen)

Keywords: portgroup added

Add keyword portgroup, to pg-related tickets

Note: See TracTickets for help on using tickets.