Remove code which corrupts versions #388

olafhering · 2021-01-13T08:55:51Z

A _service file which intends to use @PARENT_TAG@ for the package
version, and which uses versionrewrite-pattern to process the tag
further, will be unable to process the full tag name to extract the
desired parts of it via versionrewrite-replacement. The reason is the
forced removal of all hyphens.

It is up to the author of the _service file how the final package
version is assembled. versionrewrite-pattern and
versionrewrite-replacement give control how this needs to be done.

Remove all of the offending code, which should have been removed along
with the changes that introduced versionrewrite in commit
960d0ce.

Fixes issue #367

Signed-off-by: Olaf Hering [email protected]

houndci-bot · 2021-01-13T10:42:37Z

TarSCM/tasks.py

        """Automatic detection of version number for checked-out repository."""

-        version = self.scm_object.detect_version(self.args.__dict__).strip()
+        version = self.scm_object.detect_version(self.args.__dict__, truncate_at_hyphen).strip()


line too long (96 > 79 characters)

A _service file which intends to use @PARENT_TAG@ for the package version, and which uses versionrewrite-pattern to process the tag further, will be unable to process the full tag name to extract the desired parts of it via versionrewrite-replacement. The reason is the forced removal of all hyphens. It is up to the author of the _service file how the final package version is assembled. versionrewrite-pattern and versionrewrite-replacement give control how this needs to be done. Adjust detect_version to keep hyphens if the _service file uses versionrewrite-pattern, which should have been done along with the changes that introduced versionrewrite in commit 960d0ce. Fixes issue #367 Signed-off-by: Olaf Hering <[email protected]>

hramrach · 2021-01-25T10:11:24Z

Any progress here?

The failing test is python2 based. Is it even maintained? The failure looks like test setup problem.

olafhering · 2021-01-25T10:24:48Z

I can probably fix this by passing the property not as argument, but stuffing it somewhere else so that the actual consumers can grab it.

Any idea what place that would be?

M0ses · 2021-01-25T11:06:32Z

The failing test is python2 based. Is it even maintained?

short answer: yes.
obs-service-tar_scm has to run (at least) on all maintained versions *SUSE versions (because of buildtime services).

The failure looks like test setup problem.

Why? Because of pyhton2 support?

M0ses · 2021-01-25T11:10:02Z

I can probably fix this by passing the property not as argument, but stuffing it somewhere else so that the actual consumers can grab it.

Any idea what place that would be?

I`m wondering if this should be something which the user should be able to configure, in this case a new parameter would be needed. @adrianschroeter : What do you think.

BTW: test cases would be useful to understand the problem better.

hramrach · 2021-01-25T11:18:58Z

Also it does not work. I applied the patch and I get version with a hyphen:

Index: mlxbf-bootctl.obsinfo
===================================================================
--- mlxbf-bootctl.obsinfo (revision 338286a3367c7b644f23bce59dabeff9)
+++ mlxbf-bootctl.obsinfo (working copy)
@@ -1,5 +1,5 @@
name: mlxbf-bootctl
-version: 1.16.11
+version: mlxbf-bootctl-1.1-6.11
mtime: 1601841854
commit: b6dd3501668c45020f1e81690cd53aa6e21289b0

Index: mlxbf-bootctl.spec
===================================================================
--- mlxbf-bootctl.spec (revision 338286a3367c7b644f23bce59dabeff9)
+++ mlxbf-bootctl.spec (working copy)
@@ -18,7 +18,7 @@


Name: mlxbf-bootctl
-Version: 1.16.11
+Version: mlxbf-bootctl-1.1-6.11
Release: 0
Summary: User-space driver for Mellanox BlueField SoC
License: BSD-2-Clause

hramrach · 2021-01-25T11:25:19Z

Also it breaks vesionrewrite-pattern

<!-- See https://en.opensuse.org/openSUSE:Build_Service_Concept_SourceService -->
<!-- for more details on the syntax -->

<services>
        <service name="obs_scm" mode="disabled">
                <param name="scm">git</param>
                <param name="url">https://github.com/Mellanox/mlxbf-bootctl.git</param>
                <param name="filename">mlxbf-bootctl</param>
                <param name="versionrewrite-pattern">mlxbfbootctl(.*)</param>
                <param name="versionformat">@PARENT_TAG@.@TAG_OFFSET@</param>
        </service>
        <service mode="disabled" name="set_version" />
        <service name="tar" mode="buildtime"/>
</services>

olafhering · 2021-01-25T11:36:24Z

What does it break? What is the input, what is the expected output?

The goal is to pass the tag verbatim into versionrewrite-pattern.

hramrach · 2021-01-25T11:37:25Z

Nevermind, the tag was previously butchered by removing hyphens and now it is not so the pattern must be amended to also contain the hyphens.

olafhering · 2021-01-25T11:38:36Z

I`m wondering if this should be something which the user should be able to configure, in this case a new parameter would be needed. @adrianschroeter : What do you think.

You mean a I really mean it knob? Usage of versionrewrite-pattern and versionrewrite-replacement is already a clear instruction what is expected to happen.

M0ses · 2021-01-25T11:45:07Z

I`m wondering if this should be something which the user should be able to configure, in this case a new parameter would be needed. @adrianschroeter : What do you think.

You mean a I really mean it knob? Usage of versionrewrite-pattern and versionrewrite-replacement is already a clear instruction what is expected to happen.

Could you please explain the input and the expected output, in best case by a test case? I think I didn`t get the point of your commit.

olafhering · 2021-01-25T11:48:49Z

It must be possible to extract the individual parts of the verbatim tag. It is not up to tar_scm to decide if a hyphen is bad or not. That is entirely the responsibility of the _service file author, and a task of versionrewrite-pattern and versionrewrite-replacement.

hramrach · 2021-01-25T11:52:02Z

Indeed, it works with this service file:

<!-- See https://en.opensuse.org/openSUSE:Build_Service_Concept_SourceService -->
<!-- for more details on the syntax -->

<services>
        <service name="obs_scm" mode="disabled">
                <param name="scm">git</param>
                <param name="url">https://github.com/Mellanox/mlxbf-bootctl.git</param>
                <param name="filename">mlxbf-bootctl</param>
                <param name="versionrewrite-pattern">mlxbf-bootctl-(.*)-(.*)</param>
                <param name="versionrewrite-replacement">\1.\2</param>
                <param name="versionformat">@PARENT_TAG@.@TAG_OFFSET@</param>
        </service>
        <service mode="disabled" name="set_version" />
        <service name="tar" mode="buildtime"/>
</services>

Nonetheless, removing hyphens was done to prevent invalid version, and it does not happen automatically anymore. This could be seen as a regression. Maybe an error should be reported when the resulting version contains a hyphen?

olafhering · 2021-01-25T11:55:24Z

rpm will report an error on bogus input.

hramrach · 2021-01-25T12:03:19Z

I`m wondering if this should be something which the user should be able to configure, in this case a new parameter would be needed. @adrianschroeter : What do you think.

You mean a I really mean it knob? Usage of versionrewrite-pattern and versionrewrite-replacement is already a clear instruction what is expected to happen.

Could you please explain the input and the expected output, in best case by a test case? I think I didn`t get the point of your commit.

The problem is that upstream uses Debian style release tags such as 1.1-6 but does not adhere to the Debian policy to include only packaging changes in release (-n) and general changes in version (1.1). Hence the -n is significant. tar_scm further slaps the number of commits since the tag at the end 1.1-6.11 (as requested) and silently removes hyphens without any user control -> 1.16.11. This is clearly bogus. Trivially replacing the hyphen with dot gives valid version 1.1.6.11 but there is no way to achieve that with tar_scm.

hramrach · 2021-01-25T12:04:18Z

rpm will report an error on bogus input.
Clearly there was an attempt to mitigate this in the service itself before that happens.

M0ses · 2021-02-02T10:22:23Z

I opened another PR how I would solve the problem without the risk of tons of broken packages

#390

comments are welcome

M0ses · 2021-02-10T20:32:31Z

superseded by 2fdf897

olafhering requested review from M0ses and darix January 13, 2021 09:05

houndci-bot reviewed Jan 13, 2021

View reviewed changes

M0ses closed this Feb 10, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove code which corrupts versions #388

Remove code which corrupts versions #388

olafhering commented Jan 13, 2021

houndci-bot Jan 13, 2021

hramrach commented Jan 25, 2021

olafhering commented Jan 25, 2021

M0ses commented Jan 25, 2021

M0ses commented Jan 25, 2021

hramrach commented Jan 25, 2021 •

edited

Loading

hramrach commented Jan 25, 2021 •

edited

Loading

olafhering commented Jan 25, 2021

hramrach commented Jan 25, 2021

olafhering commented Jan 25, 2021

M0ses commented Jan 25, 2021

olafhering commented Jan 25, 2021

hramrach commented Jan 25, 2021

olafhering commented Jan 25, 2021

hramrach commented Jan 25, 2021

hramrach commented Jan 25, 2021

M0ses commented Feb 2, 2021

M0ses commented Feb 10, 2021

Remove code which corrupts versions #388

Remove code which corrupts versions #388

Conversation

olafhering commented Jan 13, 2021

houndci-bot Jan 13, 2021

Choose a reason for hiding this comment

hramrach commented Jan 25, 2021

olafhering commented Jan 25, 2021

M0ses commented Jan 25, 2021

M0ses commented Jan 25, 2021

hramrach commented Jan 25, 2021 • edited Loading

hramrach commented Jan 25, 2021 • edited Loading

olafhering commented Jan 25, 2021

hramrach commented Jan 25, 2021

olafhering commented Jan 25, 2021

M0ses commented Jan 25, 2021

olafhering commented Jan 25, 2021

hramrach commented Jan 25, 2021

olafhering commented Jan 25, 2021

hramrach commented Jan 25, 2021

hramrach commented Jan 25, 2021

M0ses commented Feb 2, 2021

M0ses commented Feb 10, 2021

hramrach commented Jan 25, 2021 •

edited

Loading

hramrach commented Jan 25, 2021 •

edited

Loading