Page MenuHomeSoftware Heritage
Differential D6895

ra: Add support for subversion external definitions
ClosedPublic
Actions

Authored by anlambert on Jan 10 2022, 2:19 PM.

Details

Reviewers
ardumont
Group Reviewers
Reviewers
Maniphest Tasks
T611: support for external definitions in the svn/subversion loader
Commits
rDLDSVN473fe145f4b7: ra: Add support for subversion external definitions
Summary

Subversion external definitions set on directories through the use of the
svn:externals property are now handled by the loader.

As with a svn export operation, externals will be attempted to be exported
in the paths they are defined. If an external is no longer valid (404),
the error will be ignored and the next one will be processed.

The implementation takes care of keeping the reconstructed repository
filesystem for a revision in sync with a svn export operation while
externals are added, updated or removed across revisions replay.

Related to T611

Depends on D6839

Test Plan

I have tested the implementation on numerous repositories from the list that
can be found in T611#75760.

Proceeding like this helped me to find edge cases that I did not think about
and fix them. I have added a dedicated test for each of them.

The implementation seems now pretty stable, I hope having covered all the weird
stuffs that can happen when dealing with external definitions in subversion.
Anyway, if some issue remains, they will be detected by the loader post_load
operation checking the reconstructed filesystem for the last loaded revision
is the same as the one obtained with a subversion export operation.

Diff Detail

Repository
rDLDSVN Subversion (SVN) loader
Branch
svn-loader-externals-support
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 26032
Build 40679: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 40678: arc lint + arc unit

Event Timeline

anlambert created this revision.Jan 10 2022, 2:19 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptJan 10 2022, 2:19 PM
anlambert edited the summary of this revision. (Show Details)Jan 10 2022, 2:19 PM
anlambert added a parent revision: D6839: utils: Add a function to parse a subversion external definition.
Harbormaster returned this revision to the author for changes because remote builds failed.Jan 10 2022, 2:19 PM
Harbormaster failed remote builds in B25880: Diff 25010!
swh-public-ci added a comment.Jan 10 2022, 2:20 PM

Build is green

Patch application report for D6895 (id=25010)

Could not rebase; Attempt merge onto 1b13c5c19e...

Updating 1b13c5c..472f631
Fast-forward
 swh/loader/svn/ra.py                | 219 +++++++++++++-
 swh/loader/svn/svn.py               |  28 +-
 swh/loader/svn/tests/test_loader.py | 575 ++++++++++++++++++++++++++++++------
 swh/loader/svn/tests/test_utils.py  | 197 ++++++++++++
 swh/loader/svn/utils.py             | 118 ++++++++
 5 files changed, 1031 insertions(+), 106 deletions(-)
Changes applied before test
commit 472f6312ca4f85846be769b65e762de6067c5de7
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Dec 8 11:43:38 2021 +0100

    ra: Add support for subversion external definitions
    
    Subversion external definitions set on directories through the use of the
    svn:externals property are now handled by the loader.
    
    As with a svn export operation, externals will be attempted to be exported
    in the paths they are defined. If an external is no longer valid (404),
    the error will be ignored and the next one will be processed.
    
    The implementation takes care of keeping the reconstructed repository
    filesystem for a revision in sync with a svn export operation while
    externals are added, updated or removed across revisions replay.
    
    Related to T611

commit 217a2d07811bd49912b776a7419e3f0135e2643c
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Dec 14 17:39:07 2021 +0100

    utils: Add a function to parse a subversion external definition
    
    Add a function to parse an external definition according to official
    specifications in order to extract or compute:
    
      - the relative path where the external should be exported
    
      - the URL of the external
    
      - the optional revision of the external to export
    
    Related to T611

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

swh-public-ci added a comment.Jan 10 2022, 2:21 PM

Build is green

Patch application report for D6895 (id=25010)

Could not rebase; Attempt merge onto 1b13c5c19e...

Updating 1b13c5c..472f631
Fast-forward
 swh/loader/svn/ra.py                | 219 +++++++++++++-
 swh/loader/svn/svn.py               |  28 +-
 swh/loader/svn/tests/test_loader.py | 575 ++++++++++++++++++++++++++++++------
 swh/loader/svn/tests/test_utils.py  | 197 ++++++++++++
 swh/loader/svn/utils.py             | 118 ++++++++
 5 files changed, 1031 insertions(+), 106 deletions(-)
Changes applied before test
commit 472f6312ca4f85846be769b65e762de6067c5de7
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Dec 8 11:43:38 2021 +0100

    ra: Add support for subversion external definitions
    
    Subversion external definitions set on directories through the use of the
    svn:externals property are now handled by the loader.
    
    As with a svn export operation, externals will be attempted to be exported
    in the paths they are defined. If an external is no longer valid (404),
    the error will be ignored and the next one will be processed.
    
    The implementation takes care of keeping the reconstructed repository
    filesystem for a revision in sync with a svn export operation while
    externals are added, updated or removed across revisions replay.
    
    Related to T611

commit 217a2d07811bd49912b776a7419e3f0135e2643c
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Dec 14 17:39:07 2021 +0100

    utils: Add a function to parse a subversion external definition
    
    Add a function to parse an external definition according to official
    specifications in order to extract or compute:
    
      - the relative path where the external should be exported
    
      - the URL of the external
    
      - the optional revision of the external to export
    
    Related to T611

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

Harbormaster completed remote builds in B25880: Diff 25010.Jan 10 2022, 2:21 PM
anlambert requested review of this revision.Jan 10 2022, 2:21 PM
vlorentz added a subscriber: vlorentz.Jan 12 2022, 12:35 PM

A comment that should probably be addressed in a future diff:

If I understand it correctly, svn:externals checks out the latest version of the code at the time of the check out.

  1. Does the incremental loader update the remote repo if the svn:externals property was not touched?
  2. When archiving existing repositories, this would include the latest version of a remote repo in all revisions, even those older than the remote repo, right? The loader should probably assume both repositories have matching clocks, and use the last remote revision whose timestamp is lower than the the local revision, when building each local revision.
vlorentz added a comment.Jan 12 2022, 12:58 PM

How do the tests check integrity of the results? (ie. there are the right revisions, the right files, etc.)

And a possible test to add:

remote commit 1, adds a file
local commit 2, adds an external on that file
remote commit 3, removes the file
local commit 4, does whatever

then check the content of directories of revisions 2 and 4

swh/loader/svn/ra.py
541

Why are they processed by a function called close?

570–572

are these directories removed? even if the loader crashes in this function.

You could use the dir argument of tempfile.mkdtemp() to create them in a workdir you can clean at the end without keeping track of all paths

swh/loader/svn/svn.py
223–232
anlambert added a comment.Jan 12 2022, 1:06 PM

Does the incremental loader update the remote repo if the svn:externals property was not touched?

In incremental mode, the subversion loader first checks if the history got altered (meaning it checks if the last archived revision id corresponds to the one obtained from the current svn export of that revision)
and do a full reload of the repository if it is the case. So if an external remote repo got modified, the loader will detect it and will do a full reload with the external updated.

When archiving existing repositories, this would include the latest version of a remote repo in all revisions, even those older than the remote repo, right? The loader should probably assume both repositories have matching clocks, and use the last remote revision whose timestamp is lower than the the local revision, when building each local revision.

The official subversion is working that way, it will not try to match a revision date for an external without a revision number specified and will always export the last revision of the external.
So I did not implement such processing as we use subversion revision export as reference to check if the reconstructed filesystem is valid.

anlambert added a comment.Jan 12 2022, 1:14 PM

How do the tests check integrity of the results? (ie. there are the right revisions, the right files, etc.)

When the check_revision parameter of the loader is set to 1, after each revision replay the reconstructed filesystem
is compared to the one obtained with a subversion export operation on that revision. If the root directory hashes are
equal, we know that we correctly processed the directories and files.

And a possible test to add:

remote commit 1, adds a file
local commit 2, adds an external on that file
remote commit 3, removes the file
local commit 4, does whatever

Externals are only set on directories, test_loader_set_externals_with_versioned_file_overlap covers the case you defined.

anlambert added inline comments.Jan 12 2022, 1:22 PM
swh/loader/svn/ra.py
541

Official subversion client processes externals after the export of all the files local to the repository.

The code in that diff does something similar by processing them when all local modifications to
a versioned directory have been done when replaying the current revision, i.e. when the close
method is called by the svn_ra API.
Indeed, as externals can overlap with versioned files, we need to proceed like this.

570–572

I also noticed that yesterday, I have another diff coming that adds an external cache to avoid exporting them again and again when a same external URL is set on different directories. It will ensure that temp directories are properly removed at the end of the loading.

vlorentz added a comment.Jan 12 2022, 1:30 PM

I see, thanks!

anlambert updated this revision to Diff 25098.Jan 12 2022, 2:55 PM

Rebase and ensure path of external root directory is stored in the external paths set.

swh-public-ci added a comment.Jan 12 2022, 2:57 PM

Build is green

Patch application report for D6895 (id=25098)

Could not rebase; Attempt merge onto 93b4f2fdd8...

Updating 93b4f2f..4db03a5
Fast-forward
 swh/loader/svn/ra.py                | 219 +++++++++++++-
 swh/loader/svn/svn.py               |  28 +-
 swh/loader/svn/tests/test_loader.py | 575 ++++++++++++++++++++++++++++++------
 swh/loader/svn/tests/test_utils.py  | 197 ++++++++++++
 swh/loader/svn/utils.py             | 118 ++++++++
 5 files changed, 1031 insertions(+), 106 deletions(-)
Changes applied before test
commit 4db03a5e5cd623f9178e23f093cb54b4e78903e3
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Dec 8 11:43:38 2021 +0100

    ra: Add support for subversion external definitions
    
    Subversion external definitions set on directories through the use of the
    svn:externals property are now handled by the loader.
    
    As with a svn export operation, externals will be attempted to be exported
    in the paths they are defined. If an external is no longer valid (404),
    the error will be ignored and the next one will be processed.
    
    The implementation takes care of keeping the reconstructed repository
    filesystem for a revision in sync with a svn export operation while
    externals are added, updated or removed across revisions replay.
    
    Related to T611

commit 2e7a0c23200404d815601dbb83bdb5bcc3c6c4be
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Dec 14 17:39:07 2021 +0100

    utils: Add a function to parse a subversion external definition
    
    Add a function to parse an external definition according to official
    specifications in order to extract or compute:
    
      - the relative path where the external should be exported
    
      - the URL of the external
    
      - the optional revision of the external to export
    
    Related to T611

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

Harbormaster completed remote builds in B25972: Diff 25098.Jan 12 2022, 2:57 PM
anlambert added a child revision: D6925: ra: Put externals in cache to avoid exporting them again.Jan 12 2022, 2:57 PM
anlambert added inline comments.Jan 12 2022, 2:58 PM
swh/loader/svn/ra.py
570–572

D6925

vlorentz added a comment.Jan 13 2022, 12:32 PM

LGTM, but I'd like an extra pair of eyes on it before landing

anlambert updated this revision to Diff 25168.Jan 14 2022, 12:10 PM

Optimize subversion export operation when dealing with externals: use the
origin URL as export parameter only if we know that some externals are
defined as relative to the repository URL and targets a path outside the
repository.

Indeed, when loading a repository from a remote dump mounted on the local
filesystem, it is better for performance to export a revision from that
local repository instead of a more costly export operation doing network
requests to the remote repository URL.

swh-public-ci added a comment.Jan 14 2022, 12:12 PM

Build is green

Patch application report for D6895 (id=25168)

Could not rebase; Attempt merge onto 93b4f2fdd8...

Updating 93b4f2f..85aa87b
Fast-forward
 swh/loader/svn/ra.py                | 235 ++++++++++++-
 swh/loader/svn/svn.py               |  45 ++-
 swh/loader/svn/tests/test_loader.py | 647 +++++++++++++++++++++++++++++++-----
 swh/loader/svn/tests/test_utils.py  | 225 ++++++++++++-
 swh/loader/svn/utils.py             | 126 ++++++-
 5 files changed, 1168 insertions(+), 110 deletions(-)
Changes applied before test
commit 85aa87be50fea493c437b44cbcc544f285912e5d
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Dec 8 11:43:38 2021 +0100

    ra: Add support for subversion external definitions
    
    Subversion external definitions set on directories through the use of the
    svn:externals property are now handled by the loader.
    
    As with a svn export operation, externals will be attempted to be exported
    in the paths they are defined. If an external is no longer valid (404),
    the error will be ignored and the next one will be processed.
    
    The implementation takes care of keeping the reconstructed repository
    filesystem for a revision in sync with a svn export operation while
    externals are added, updated or removed across revisions replay.
    
    Related to T611

commit 2f5fd60ab91f5af90c3333f369cb7b72b28b3fbe
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Dec 14 17:39:07 2021 +0100

    utils: Add a function to parse a subversion external definition
    
    Add a function to parse an external definition according to official
    specifications in order to extract or compute:
    
      - the relative path where the external should be exported
    
      - the URL of the external
    
      - the optional revision of the external to export
    
    Related to T611

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

Harbormaster completed remote builds in B26032: Diff 25168.Jan 14 2022, 12:12 PM
anlambert updated this revision to Diff 25172.Jan 14 2022, 12:52 PM
  • Use context manager to create temporary directory for checkout
  • Update years in test_loader.py license header
swh-public-ci added a comment.Jan 14 2022, 12:54 PM

Build is green

Patch application report for D6895 (id=25172)

Could not rebase; Attempt merge onto 93b4f2fdd8...

Updating 93b4f2f..8c7046e
Fast-forward
 swh/loader/svn/ra.py                | 235 ++++++++++++-
 swh/loader/svn/svn.py               |  44 ++-
 swh/loader/svn/tests/test_loader.py | 650 +++++++++++++++++++++++++++++++-----
 swh/loader/svn/tests/test_utils.py  | 225 ++++++++++++-
 swh/loader/svn/utils.py             | 126 ++++++-
 5 files changed, 1169 insertions(+), 111 deletions(-)
Changes applied before test
commit 8c7046e0ab03ae4b8f93134ff0d85d80c4352c69
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Dec 8 11:43:38 2021 +0100

    ra: Add support for subversion external definitions
    
    Subversion external definitions set on directories through the use of the
    svn:externals property are now handled by the loader.
    
    As with a svn export operation, externals will be attempted to be exported
    in the paths they are defined. If an external is no longer valid (404),
    the error will be ignored and the next one will be processed.
    
    The implementation takes care of keeping the reconstructed repository
    filesystem for a revision in sync with a svn export operation while
    externals are added, updated or removed across revisions replay.
    
    Related to T611

commit 2f5fd60ab91f5af90c3333f369cb7b72b28b3fbe
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Dec 14 17:39:07 2021 +0100

    utils: Add a function to parse a subversion external definition
    
    Add a function to parse an external definition according to official
    specifications in order to extract or compute:
    
      - the relative path where the external should be exported
    
      - the URL of the external
    
      - the optional revision of the external to export
    
    Related to T611

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

Harbormaster completed remote builds in B26036: Diff 25172.Jan 14 2022, 12:54 PM
anlambert updated this revision to Diff 25176.Jan 14 2022, 1:17 PM

Also filter out empty lines and commented ones when parsing external
definitions after a checkout.

swh-public-ci added a comment.Jan 14 2022, 1:19 PM

Build is green

Patch application report for D6895 (id=25176)

Could not rebase; Attempt merge onto 93b4f2fdd8...

Updating 93b4f2f..e76adb1
Fast-forward
 swh/loader/svn/ra.py                | 235 ++++++++++++-
 swh/loader/svn/svn.py               |  47 ++-
 swh/loader/svn/tests/test_loader.py | 650 +++++++++++++++++++++++++++++++-----
 swh/loader/svn/tests/test_utils.py  | 225 ++++++++++++-
 swh/loader/svn/utils.py             | 126 ++++++-
 5 files changed, 1172 insertions(+), 111 deletions(-)
Changes applied before test
commit e76adb16d51b726eb0a3c5b266579eeba566f48d
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Dec 8 11:43:38 2021 +0100

    ra: Add support for subversion external definitions
    
    Subversion external definitions set on directories through the use of the
    svn:externals property are now handled by the loader.
    
    As with a svn export operation, externals will be attempted to be exported
    in the paths they are defined. If an external is no longer valid (404),
    the error will be ignored and the next one will be processed.
    
    The implementation takes care of keeping the reconstructed repository
    filesystem for a revision in sync with a svn export operation while
    externals are added, updated or removed across revisions replay.
    
    Related to T611

commit 2f5fd60ab91f5af90c3333f369cb7b72b28b3fbe
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Dec 14 17:39:07 2021 +0100

    utils: Add a function to parse a subversion external definition
    
    Add a function to parse an external definition according to official
    specifications in order to extract or compute:
    
      - the relative path where the external should be exported
    
      - the URL of the external
    
      - the optional revision of the external to export
    
    Related to T611

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

Harbormaster completed remote builds in B26040: Diff 25176.Jan 14 2022, 1:19 PM
ardumont added a subscriber: ardumont.Jan 17 2022, 2:38 PM

lgtm but I'm not sure i understood everything (code wise).

Let's land and check on staging that all is fine on the list you mentioned in the test plan?

I think it'd be good we start thinking about documenting our loaders.
Because for example, the svn:external is rather complex to follow through the code.
Definitely starting the svn:external with your great task comment [1]

[1] T611#75760

swh/loader/svn/ra.py
591

pass can be dropped as there are other instructions.

640
673

what kind of error can arise here?
Don't we want to log it?

swh/loader/svn/tests/test_loader.py
1915

How are they invalid? Is it because the remote url is a file:// one?

ardumont accepted this revision.Jan 17 2022, 2:38 PM
This revision is now accepted and ready to land.Jan 17 2022, 2:38 PM
anlambert updated this revision to Diff 25229.Jan 18 2022, 10:59 AM

Update:

  • Improve test when all externals got removed from a path
  • Ensure directory where externals are defined exists before exporting externals
swh-public-ci added a comment.Jan 18 2022, 11:01 AM

Build is green

Patch application report for D6895 (id=25229)

Could not rebase; Attempt merge onto cb4bf60c0e...

Merge made by the 'recursive' strategy.
 swh/loader/svn/ra.py                | 237 ++++++++++++-
 swh/loader/svn/svn.py               |  47 ++-
 swh/loader/svn/tests/test_loader.py | 650 +++++++++++++++++++++++++++++++-----
 swh/loader/svn/tests/test_utils.py  | 261 ++++++++++++++-
 swh/loader/svn/utils.py             | 128 ++++++-
 5 files changed, 1212 insertions(+), 111 deletions(-)
Changes applied before test
commit 02bb23e1d35cc1cfcdc0fc430f6e1844208e5adc
Merge: cb4bf60 aba4e6e
Author: Jenkins user <jenkins@localhost>
Date:   Tue Jan 18 09:59:22 2022 +0000

    Merge branch 'diff-target' into HEAD

commit aba4e6e29600cc6b4e4a4c686ff4946b5ce9b077
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Dec 8 11:43:38 2021 +0100

    ra: Add support for subversion external definitions
    
    Subversion external definitions set on directories through the use of the
    svn:externals property are now handled by the loader.
    
    As with a svn export operation, externals will be attempted to be exported
    in the paths they are defined. If an external is no longer valid (404),
    the error will be ignored and the next one will be processed.
    
    The implementation takes care of keeping the reconstructed repository
    filesystem for a revision in sync with a svn export operation while
    externals are added, updated or removed across revisions replay.
    
    Related to T611

commit 30b3c8427391edc0f88dce202dcb8abb07bf548b
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Dec 14 17:39:07 2021 +0100

    utils: Add a function to parse a subversion external definition
    
    Add a function to parse an external definition according to official
    specifications in order to extract or compute:
    
      - the relative path where the external should be exported
    
      - the URL of the external
    
      - the optional revision of the external to export
    
    Related to T611

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

Harbormaster completed remote builds in B26090: Diff 25229.Jan 18 2022, 11:01 AM
anlambert updated this revision to Diff 25238.Jan 18 2022, 11:10 AM

Rebase

swh-public-ci added a comment.Jan 18 2022, 11:11 AM

Build is green

Patch application report for D6895 (id=25238)

Could not rebase; Attempt merge onto cb4bf60c0e...

Updating cb4bf60..13eb16e
Fast-forward
 swh/loader/svn/ra.py                | 237 ++++++++++++-
 swh/loader/svn/svn.py               |  47 ++-
 swh/loader/svn/tests/test_loader.py | 650 +++++++++++++++++++++++++++++++-----
 swh/loader/svn/tests/test_utils.py  | 261 ++++++++++++++-
 swh/loader/svn/utils.py             | 128 ++++++-
 5 files changed, 1212 insertions(+), 111 deletions(-)
Changes applied before test
commit 13eb16e499e79b7a5914af4961cdc5afeea9eada
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Dec 8 11:43:38 2021 +0100

    ra: Add support for subversion external definitions
    
    Subversion external definitions set on directories through the use of the
    svn:externals property are now handled by the loader.
    
    As with a svn export operation, externals will be attempted to be exported
    in the paths they are defined. If an external is no longer valid (404),
    the error will be ignored and the next one will be processed.
    
    The implementation takes care of keeping the reconstructed repository
    filesystem for a revision in sync with a svn export operation while
    externals are added, updated or removed across revisions replay.
    
    Related to T611

commit f1913512a5faa0c99d23607b9d63fc6003c729fb
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Dec 14 17:39:07 2021 +0100

    utils: Add a function to parse a subversion external definition
    
    Add a function to parse an external definition according to official
    specifications in order to extract or compute:
    
      - the relative path where the external should be exported
    
      - the URL of the external
    
      - the optional revision of the external to export
    
    Related to T611

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

Harbormaster completed remote builds in B26099: Diff 25238.Jan 18 2022, 11:12 AM
anlambert added a comment.Jan 18 2022, 11:18 AM

lgtm but I'm not sure i understood everything (code wise).

Yes, dealing with externals when replaying revisions is quite complex.
The simplest solution would have been to export all revisions with the subversion client
instead of replaying them but as you know this is awful in terms of performance.

Let's land and check on staging that all is fine on the list you mentioned in the test plan?

Good idea, I will update the repos list in the task as new ones got processed by the loader
in production.

I think it'd be good we start thinking about documenting our loaders.
Because for example, the svn:external is rather complex to follow through the code.
Definitely starting the svn:external with your great task comment [1]

That's part of my roadmap for 2022 ;-)

swh/loader/svn/tests/test_loader.py
1915

They are invalid because the external URLs target non existing paths (404 simulation).

anlambert added inline comments.Jan 18 2022, 11:22 AM
swh/loader/svn/ra.py
591

right

673

There is few chances that a versioned path overlaps an external but cases exist.

We do not log the error here as it means there is no overlap between versioned path and external.

anlambert updated this revision to Diff 25241.Jan 18 2022, 11:25 AM

Remove not needed pass instruction

swh-public-ci added a comment.Jan 18 2022, 11:27 AM

Build is green

Patch application report for D6895 (id=25241)

Could not rebase; Attempt merge onto cb4bf60c0e...

Updating cb4bf60..9dd1921
Fast-forward
 swh/loader/svn/ra.py                | 236 ++++++++++++-
 swh/loader/svn/svn.py               |  47 ++-
 swh/loader/svn/tests/test_loader.py | 650 +++++++++++++++++++++++++++++++-----
 swh/loader/svn/tests/test_utils.py  | 261 ++++++++++++++-
 swh/loader/svn/utils.py             | 128 ++++++-
 5 files changed, 1211 insertions(+), 111 deletions(-)
Changes applied before test
commit 9dd1921a9935d118bd97645b2f6844b14db536db
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Dec 8 11:43:38 2021 +0100

    ra: Add support for subversion external definitions
    
    Subversion external definitions set on directories through the use of the
    svn:externals property are now handled by the loader.
    
    As with a svn export operation, externals will be attempted to be exported
    in the paths they are defined. If an external is no longer valid (404),
    the error will be ignored and the next one will be processed.
    
    The implementation takes care of keeping the reconstructed repository
    filesystem for a revision in sync with a svn export operation while
    externals are added, updated or removed across revisions replay.
    
    Related to T611

commit f1913512a5faa0c99d23607b9d63fc6003c729fb
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Dec 14 17:39:07 2021 +0100

    utils: Add a function to parse a subversion external definition
    
    Add a function to parse an external definition according to official
    specifications in order to extract or compute:
    
      - the relative path where the external should be exported
    
      - the URL of the external
    
      - the optional revision of the external to export
    
    Related to T611

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

Harbormaster completed remote builds in B26102: Diff 25241.Jan 18 2022, 11:27 AM
anlambert updated this revision to Diff 25248.Jan 18 2022, 12:40 PM

Update: Add missing root path check in DirEditor.add_directory

swh-public-ci added a comment.Jan 18 2022, 12:42 PM

Build is green

Patch application report for D6895 (id=25248)

Could not rebase; Attempt merge onto cb4bf60c0e...

Updating cb4bf60..1df45e9
Fast-forward
 swh/loader/svn/ra.py                | 236 ++++++++++++-
 swh/loader/svn/svn.py               |  47 ++-
 swh/loader/svn/tests/test_loader.py | 650 +++++++++++++++++++++++++++++++-----
 swh/loader/svn/tests/test_utils.py  | 261 ++++++++++++++-
 swh/loader/svn/utils.py             | 128 ++++++-
 5 files changed, 1211 insertions(+), 111 deletions(-)
Changes applied before test
commit 1df45e92b87852cfd3badc48accd1ea0a5b8ae9a
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Dec 8 11:43:38 2021 +0100

    ra: Add support for subversion external definitions
    
    Subversion external definitions set on directories through the use of the
    svn:externals property are now handled by the loader.
    
    As with a svn export operation, externals will be attempted to be exported
    in the paths they are defined. If an external is no longer valid (404),
    the error will be ignored and the next one will be processed.
    
    The implementation takes care of keeping the reconstructed repository
    filesystem for a revision in sync with a svn export operation while
    externals are added, updated or removed across revisions replay.
    
    Related to T611

commit f1913512a5faa0c99d23607b9d63fc6003c729fb
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Dec 14 17:39:07 2021 +0100

    utils: Add a function to parse a subversion external definition
    
    Add a function to parse an external definition according to official
    specifications in order to extract or compute:
    
      - the relative path where the external should be exported
    
      - the URL of the external
    
      - the optional revision of the external to export
    
    Related to T611

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

Harbormaster completed remote builds in B26110: Diff 25248.Jan 18 2022, 12:42 PM
anlambert updated this revision to Diff 25249.Jan 18 2022, 12:45 PM

Fix wrong rebase

swh-public-ci added a comment.Jan 18 2022, 12:47 PM

Build is green

Patch application report for D6895 (id=25249)

Could not rebase; Attempt merge onto cb4bf60c0e...

Updating cb4bf60..473fe14
Fast-forward
 swh/loader/svn/ra.py                | 236 ++++++++++++-
 swh/loader/svn/svn.py               |  47 ++-
 swh/loader/svn/tests/test_loader.py | 650 +++++++++++++++++++++++++++++++-----
 swh/loader/svn/tests/test_utils.py  | 261 ++++++++++++++-
 swh/loader/svn/utils.py             | 128 ++++++-
 5 files changed, 1211 insertions(+), 111 deletions(-)
Changes applied before test
commit 473fe145f4b7cd6cd1e1c0f9ec72cad04c38c4a8
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Dec 8 11:43:38 2021 +0100

    ra: Add support for subversion external definitions
    
    Subversion external definitions set on directories through the use of the
    svn:externals property are now handled by the loader.
    
    As with a svn export operation, externals will be attempted to be exported
    in the paths they are defined. If an external is no longer valid (404),
    the error will be ignored and the next one will be processed.
    
    The implementation takes care of keeping the reconstructed repository
    filesystem for a revision in sync with a svn export operation while
    externals are added, updated or removed across revisions replay.
    
    Related to T611

commit f1913512a5faa0c99d23607b9d63fc6003c729fb
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Dec 14 17:39:07 2021 +0100

    utils: Add a function to parse a subversion external definition
    
    Add a function to parse an external definition according to official
    specifications in order to extract or compute:
    
      - the relative path where the external should be exported
    
      - the URL of the external
    
      - the optional revision of the external to export
    
    Related to T611

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

Harbormaster completed remote builds in B26111: Diff 25249.Jan 18 2022, 12:47 PM
Closed by commit rDLDSVN473fe145f4b7: ra: Add support for subversion external definitions (authored by anlambert). · Explain WhyJan 18 2022, 12:50 PM
This revision was automatically updated to reflect the committed changes.
anlambert added a commit: rDLDSVN473fe145f4b7: ra: Add support for subversion external definitions.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
loader/
svn/
235 lines91%
Loading...
45 lines97%
Loading...
tests/
647 lines99%
Loading...

Diff 25168

View Options

swh/loader/svn/ra.py

Loading...
View Options

swh/loader/svn/svn.py

Loading...
View Options

swh/loader/svn/tests/test_loader.py

Loading...
About · Developer information · Legal