Page MenuHomeSoftware Heritage
Differential D6749

debian: Remove the extrinsic version from release names
ClosedPublic
Actions

Authored by vlorentz on Dec 6 2021, 2:06 PM.

Details

Reviewers
olasd
anlambert
Group Reviewers
Reviewers
Commits
rDLDBASEc87c8c79ea66: debian: Remove the extrinsic version from release names
Summary

Use only the intrinsic version (eg. 1.0.0) instead of the extrinsic version
(eg. stretch/contrib/1.0.0).
Releases should only contain data from DSC, not external 'pointers' to them.

Additionally, having extrinsic data in releases means the same
dsc-sha256 extid can point to different releases, which meant the loader
may reuse a release mentioning a specific suite as a release in a
different suite.
With this commit, this won't be a problem anymore, as releases won't
mention the suite at all, so suites can safely share extids.

Depends on D6748

Note: this diff fixes data corruption, that at best crashes the loaders,
at worst makes them write wrong stuff. In order to fix it, we need to wipe
all ExtIDs from the database with type dsc-sha256 pointing to a release SWHID.

Diff Detail

Event Timeline

vlorentz created this revision.Dec 6 2021, 2:06 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptDec 6 2021, 2:06 PM
vlorentz added a reviewer: olasd.Dec 6 2021, 2:06 PM
vlorentz edited the summary of this revision. (Show Details)
vlorentz edited the test plan for this revision. (Show Details)
swh-public-ci added a comment.Dec 6 2021, 2:08 PM

Build is green

Patch application report for D6749 (id=24510)

Could not rebase; Attempt merge onto 5d22455c94...

Updating 5d22455..2e01f10
Fast-forward
 docs/package-loader-specifications.rst         |  9 ++++--
 swh/loader/package/debian/loader.py            | 19 +++++++----
 swh/loader/package/debian/tests/test_debian.py | 44 +++++++++++++++++---------
 3 files changed, 47 insertions(+), 25 deletions(-)
Changes applied before test
commit 2e01f10bb05215ed724d490415e8cce9ccd89b35
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Dec 6 14:03:51 2021 +0100

    debian: Remove the extrinsic version from release names
    
    Use only the intrinsic version (eg. 1.0.0) instead of the extrinsic version
    (eg. stretch/contrib/1.0.0).
    Releases should only contain data from DSC, not external 'pointers' to them.
    
    Additionally, having extrinsic data in releases means the same
    dsc-sha256 extid can point to different releases, which meant the loader
    may reuse a release mentioning a specific suite as a release in a
    different suite.
    With this commit, this won't be a problem anymore, as releases won't
    mention the suite at all, so suites can safely share extids.

commit 958674dc24171ced169397054ffb1502b60f7f43
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Dec 6 13:48:04 2021 +0100

    debian: Fix confusion between the two versions
    
    'version' was documented as the intrinsic version (eg. '0.7.2-3') and
    'full_version' as the one containing the suite name (eg. 'stretch/contrib/0.7.2-3').
    
    In practice, it was the opposite, except in a few incorrect test.
    
    This commit fixes said tests, and renamed 'full_version' to
    'intrinsic_version'.
    
    This is only a refactoring, the behavior is unchanged for now;
    but a future commit will remove the 'version' (which is extrinsic) from
    the release name (which should contain only data intrinsic to the DSC).

See https://jenkins.softwareheritage.org/job/DLDBASE/job/tests-on-diff/644/ for more details.

Harbormaster completed remote builds in B25384: Diff 24510.Dec 6 2021, 2:08 PM
vlorentz requested review of this revision.Dec 6 2021, 2:08 PM
anlambert accepted this revision.Dec 6 2021, 2:44 PM
anlambert added a subscriber: anlambert.

Looks good to me.

Indeed, releases with the same intrinsic version target the same directory regardless of the suite (see example below before that diff) so nice catch !

olasd accepted this revision.Dec 6 2021, 2:47 PM

Thanks for fixing this.

I'm not too sure about wiping the extids. Sounds like a case for bumping the expected extid_version instead.

This revision is now accepted and ready to land.Dec 6 2021, 2:47 PM
vlorentz added a comment.Dec 6 2021, 2:49 PM

I'm not too sure about wiping the extids. Sounds like a case for bumping the expected extid_version instead.

Bumping the extid_version means we need to reload Debian entirely from scratch. By just wiping these release extids, we'll only discard what was loaded in the last couple of weeks.

olasd added a comment.Dec 6 2021, 2:50 PM
In D6749#175555, @vlorentz wrote:

I'm not too sure about wiping the extids. Sounds like a case for bumping the expected extid_version instead.

Bumping the extid_version means we need to reload Debian entirely from scratch.

Yes.

By just wiping these release extids, we'll only discard what was loaded in the last couple of weeks.

That is much, much more annoying to do.

ardumont mentioned this in T3774: Deploy swh.loader.core v2.0.0.Dec 7 2021, 9:29 AM
vlorentz updated this revision to Diff 24538.Dec 7 2021, 11:06 AM

rebase

swh-public-ci added a comment.Dec 7 2021, 11:09 AM

Build has FAILED

Patch application report for D6749 (id=24538)

Could not rebase; Attempt merge onto 2d9e93a2f2...

Updating 2d9e93a..4632adb
Fast-forward
 docs/package-loader-specifications.rst         |  9 ++++--
 swh/loader/package/debian/loader.py            | 19 +++++++----
 swh/loader/package/debian/tests/test_debian.py | 44 +++++++++++++++++---------
 3 files changed, 47 insertions(+), 25 deletions(-)
Changes applied before test
commit 4632adb5af307dc48d791276a12e8aafa46975de
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Dec 6 14:03:51 2021 +0100

    debian: Remove the extrinsic version from release names
    
    Use only the intrinsic version (eg. 1.0.0) instead of the extrinsic version
    (eg. stretch/contrib/1.0.0).
    Releases should only contain data from DSC, not external 'pointers' to them.
    
    Additionally, having extrinsic data in releases means the same
    dsc-sha256 extid can point to different releases, which meant the loader
    may reuse a release mentioning a specific suite as a release in a
    different suite.
    With this commit, this won't be a problem anymore, as releases won't
    mention the suite at all, so suites can safely share extids.

commit 26996ef3eef494eb187e01b999255cfa101ad758
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Dec 6 13:48:04 2021 +0100

    debian: Fix confusion between the two versions
    
    'version' was documented as the intrinsic version (eg. '0.7.2-3') and
    'full_version' as the one containing the suite name (eg. 'stretch/contrib/0.7.2-3').
    
    In practice, it was the opposite, except in a few incorrect test.
    
    This commit fixes said tests, and renamed 'full_version' to
    'intrinsic_version'.
    
    This is only a refactoring, the behavior is unchanged for now;
    but a future commit will remove the 'version' (which is extrinsic) from
    the release name (which should contain only data intrinsic to the DSC).

Link to build: https://jenkins.softwareheritage.org/job/DLDBASE/job/tests-on-diff/650/
See console output for more information: https://jenkins.softwareheritage.org/job/DLDBASE/job/tests-on-diff/650/console

Harbormaster failed remote builds in B25412: Diff 24538!Dec 7 2021, 11:09 AM
vlorentz updated this revision to Diff 24541.Dec 7 2021, 11:25 AM

fix rebase (missing test update)

swh-public-ci added a comment.Dec 7 2021, 11:28 AM

Build is green

Patch application report for D6749 (id=24541)

Could not rebase; Attempt merge onto 2d9e93a2f2...

Updating 2d9e93a..c87c8c7
Fast-forward
 docs/package-loader-specifications.rst         |  9 +++--
 swh/loader/package/debian/loader.py            | 19 ++++++----
 swh/loader/package/debian/tests/test_debian.py | 50 ++++++++++++++++----------
 3 files changed, 50 insertions(+), 28 deletions(-)
Changes applied before test
commit c87c8c79ea660d5de697455d7c1882a216b23df5
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Dec 6 14:03:51 2021 +0100

    debian: Remove the extrinsic version from release names
    
    Use only the intrinsic version (eg. 1.0.0) instead of the extrinsic version
    (eg. stretch/contrib/1.0.0).
    Releases should only contain data from DSC, not external 'pointers' to them.
    
    Additionally, having extrinsic data in releases means the same
    dsc-sha256 extid can point to different releases, which meant the loader
    may reuse a release mentioning a specific suite as a release in a
    different suite.
    With this commit, this won't be a problem anymore, as releases won't
    mention the suite at all, so suites can safely share extids.

commit 26996ef3eef494eb187e01b999255cfa101ad758
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Dec 6 13:48:04 2021 +0100

    debian: Fix confusion between the two versions
    
    'version' was documented as the intrinsic version (eg. '0.7.2-3') and
    'full_version' as the one containing the suite name (eg. 'stretch/contrib/0.7.2-3').
    
    In practice, it was the opposite, except in a few incorrect test.
    
    This commit fixes said tests, and renamed 'full_version' to
    'intrinsic_version'.
    
    This is only a refactoring, the behavior is unchanged for now;
    but a future commit will remove the 'version' (which is extrinsic) from
    the release name (which should contain only data intrinsic to the DSC).

See https://jenkins.softwareheritage.org/job/DLDBASE/job/tests-on-diff/652/ for more details.

Harbormaster completed remote builds in B25415: Diff 24541.Dec 7 2021, 11:28 AM
Closed by commit rDLDBASEc87c8c79ea66: debian: Remove the extrinsic version from release names (authored by vlorentz). · Explain WhyDec 7 2021, 1:26 PM
This revision was automatically updated to reflect the committed changes.
vlorentz added a commit: rDLDBASEc87c8c79ea66: debian: Remove the extrinsic version from release names.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
docs/
2 lines--
swh/
loader/
package/
debian/
2 lines93%
Loading...
tests/
22 lines100%
Loading...

Diff 24541

View Options

docs/package-loader-specifications.rst

Loading...
View Options

swh/loader/package/debian/loader.py

Loading...
View Options

swh/loader/package/debian/tests/test_debian.py

Loading...
About · Developer information · Legal