Make version parsing work for ILDConfig tags

[tmadlener]

A few tags in ILDConfig have an ever so slightly different format, using p instead of pre, which breaks the tagging script.

BEGINRELEASENOTES

• Make the tagging script no longer error for ILDConfig patch releases with a pXY patch version specifier. NOTE: This only makes the tagging script ignore such tags. It does not handle them on the same footing as other versions.

ENDRELEASENOTES

[andresailer]

Those ILDConfig tags with a p aren't pre-releases though, are they?

[andresailer]

Suggested change

	self.patch = int( parts[2] )
	self.patch = int( parts[2].strip('p') )

Would this by itself solve the ILDConfig problem?

[tmadlener]

Yes, indeed that would work as a minimal solution as well. But that basically ignores p releases(?). At least the following breaks (but it also does with all other changes in place)

python ilcsofttagger.py --lastTag v02-02-p01 --packages ILDConfig

[andresailer]

In what way does it break? Is it because there is also 02-02-01 (https://github.com/iLCSoft/ILDConfig/releases/tag/v02-02-01)

[tmadlener]

This is not strictly what initially triggered this pull request, and a bit contrived for the current situation, but if p releases should be considered patch releases they should be usable as arguments to --lastTag I think.

So with the following

python ilcsofttagger.py --lastTag v02-02-p01 --ignoreMissingCmake --properRelease --packages ILDConfig

I get a parsing error without any changes (this is what I intended to fix with this pull request):

INFO  - Tagger  : Checking package: ILDConfig
ERROR - Version : Failed to parse version: invalid literal for int() with base 10: 'p02'
ERROR - Tagger  : Failure for iLCSoft/ILDConfig/master/None
ERROR - Tagger  : ERROR: broken version string
ERROR - root    : Error during runtime: ERROR

Including the minimal fix with stripping the p I get:

INFO  - Tagger  : Checking package: ILDConfig
INFO  - ILDConfig: Found latest tag v02-02-p01
INFO  - ILDConfig: Set new version: v02-02-02
ERROR - ILDConfig: Required (pre-)version ('v02-02-02') is the same as or older than the last tag('v02-02-p01')
ERROR - Tagger  : Failure for iLCSoft/ILDConfig/master/None
ERROR - Tagger  : ERROR: Invalid version required
ERROR - root    : Error during runtime: ERROR

Looking at the debug output, it looks like the -pXX should always populate the last field of the version tuple, currently it populates the one at which it appears, which seems to break version comparison (?).

[andresailer]

What should be the outcome of the command?

Looking at the code the issue is with

iLCInstall/scripts/tagging/helperfunctions.py

Line 37 in 7e87bd1

def versionComp( v1, v2 ):

called from

iLCInstall/scripts/tagging/gitinterface.py

Lines 94 to 99 in 7e87bd1

	if self._nextRealRelease == lastTag \
	or versionComp( self._nextRealRelease, lastTag ) <= 0 \
	or versionComp( self.newTag, lastTag ) <= 0:
	self.log.error("Required (pre-)version (%r) is the same as or older than the last tag(%r)",
	self.newVersion, lastTag)
	raise RuntimeError( "Invalid version required" )

and not the parsing of the version string.

[tmadlener]

In this case the tags are the following (already sorted):

v02-02
v02-02-p01
v02-02-p02
v02-02-01

So when I specify --lastTag v02-02-p01 ideally all changes from v02-02-p02 and v02-02-01 should be collected.

[tmadlener]

What could work in principle is to parse the pXY into the Version.pre field for comparison. The Version.isPre could in principle still be used for differentiating between pre releases and patch releases. On the other hand I am not entirely sure if is worth all the complexity for handling this arguable edge case.

[andresailer]

The real fun begins when you get versions like https://github.com/iLCSoft/ILDConfig/releases/tag/v01-19-06-p01
I wouldn't abuse pre for this, because in other places pre is used only for the pre purpose.
One could add a second patch member variable to store entries with p

[tmadlener]

Yeah, after playing with this a bit, abusing the pre field doesn't really work out, even if the parsing and sorting problems are sorted out, there are still some remaining problems with the str conversion.

So I think fixing this properly would take a bit of time. However, simply fixing the parsing, would at least allow to use the tagging script to tag ILDConfig, even if p releases are not handled completely correctly.

[tmadlener]

Those ILDConfig tags with a p aren't pre-releases though, are they?

Good question, @gaede should they be considered rather as patch releases to the config with a dedicated naming scheme that does not conflict with the usual ilcsoft versioning scheme?

[gaede]

Yes, Andres is right - only tags containing 'pre' are pre-releases. We often use sth. like -p01 as patch releases. The rationale is that the first part of the ILDConfig version should be that of the ilcsoft release that it is compatible with. (So no p01 or the like in ilcsoft versions - only ILDConfig...)

[tmadlener]

triggering CI