Page MenuHomeSoftware Heritage
Differential D6839

utils: Add a function to parse a subversion external definition
ClosedPublic
Actions

Authored by anlambert on Dec 14 2021, 5:55 PM.

Details

Reviewers
ardumont
Group Reviewers
Reviewers
Maniphest Tasks
T611: support for external definitions in the svn/subversion loader
Commits
rDLDSVNf1913512a5fa: utils: Add a function to parse a subversion external definition
Summary

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

Depends on D6838

Diff Detail

Repository
rDLDSVN Subversion (SVN) loader
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

anlambert created this revision.Dec 14 2021, 5:55 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptDec 14 2021, 5:55 PM
swh-public-ci added a comment.Dec 14 2021, 5:57 PM

Build is green

Patch application report for D6839 (id=24786)

Could not rebase; Attempt merge onto 0ae91422d8...

Updating 0ae9142..42e4b27
Fast-forward
 swh/loader/svn/loader.py           |  19 ++--
 swh/loader/svn/ra.py               | 106 ++++++++++----------
 swh/loader/svn/tests/test_utils.py | 191 +++++++++++++++++++++++++++++++++++++
 swh/loader/svn/utils.py            | 116 ++++++++++++++++++++++
 4 files changed, 366 insertions(+), 66 deletions(-)
Changes applied before test
commit 42e4b27b17a4f50aa984e76b9b7ef9a0811526ff
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

commit c56712550aa4597935e9798bceefb5c8e7b8a860
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Dec 8 19:26:13 2021 +0100

    loader: Simplify fetch_data implementation

commit f069043f70901218c13fcf08883e3f9c9e56617e
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Dec 8 17:23:22 2021 +0100

    ra: Add debug logging when setting svn:externals property on a path
    
    Related to T611

commit 6a889e041a4039b8c2a213e8b0146a48fe10b233
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Dec 8 13:40:58 2021 +0100

    ra: Add path parameter to DirEditor
    
    It enables to get the path of the currently processed directory
    which can be useful for logging and future improvements of the
    loader (svn:externals property support for instance).
    
    Related to T611

commit f6f0c5bb3703ea1b2ba271eca43e64b51948c6d2
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Dec 8 11:55:04 2021 +0100

    ra: Merge BaseDirEditor and DirEditor into a single DirEditor class
    
    DirEditor being the only class derived from the BaseDirEditor one,
    let's merge their code into a single DirEditor class.

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

Harbormaster completed remote builds in B25635: Diff 24786.Dec 14 2021, 5:57 PM
anlambert requested review of this revision.Dec 14 2021, 5:57 PM
douardda added a subscriber: douardda.Jan 4 2022, 4:12 PM

Gawd this is horrible! (not your fault!)

swh/loader/svn/utils.py
173

I don't understand the usage of 'absolute' in this docstring. Wouldn't it better to describe it as "according SVN external definitions" or so?

I mean 'join("url://path/", "/toto")' giving "url://path/toto" does not make much sense to me (being used to urllib's behavior) but qulifying this as "absolute" does not make much sense, so "join url the SVN way" seems better to me :-)

223

why does't external.split() suffice for the job?

283

I see no test for this case (url@toto), what is the expected result there?

anlambert added a comment.Jan 4 2022, 4:30 PM
In D6839#178745, @douardda wrote:

Gawd this is horrible! (not your fault!)

Yeah, I had a lot of fun writing this one but subversion externals specification can have so many forms.

swh/loader/svn/utils.py
173

Ack, I can rename it to svn_urljoin, this will be less confusing.

223

yeah right, it is enough to do so.

283

We can have some URLs like http://anon:anon@svn.example.com/skin-maker (it is in a test case), so that workaround.

anlambert updated this revision to Diff 24930.Jan 4 2022, 5:57 PM

Rebase and adapt according to @douardda reviews

swh-public-ci added a comment.Jan 4 2022, 5:58 PM

Build is green

Patch application report for D6839 (id=24930)

Rebasing onto 196c9d596b...

Current branch diff-target is up to date.
Changes applied before test
commit bf0c228d00eb32a11291047e42b7c62f9be4e779
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/215/ for more details.

Harbormaster completed remote builds in B25801: Diff 24930.Jan 4 2022, 5:59 PM
anlambert updated this revision to Diff 24931.Jan 4 2022, 6:14 PM

Fix docstring

swh-public-ci added a comment.Jan 4 2022, 6:15 PM

Build is green

Patch application report for D6839 (id=24931)

Rebasing onto 196c9d596b...

Current branch diff-target is up to date.
Changes applied before test
commit f78f71aecfda81f4518e13921e38508624f82925
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/216/ for more details.

Harbormaster completed remote builds in B25802: Diff 24931.Jan 4 2022, 6:15 PM
ardumont added 1 blocking reviewer(s): Reviewers.Jan 5 2022, 9:58 AM
ardumont accepted this revision.Jan 5 2022, 10:01 AM
ardumont added a subscriber: ardumont.

lgtm

I got the feeling the more we will dig into those old vcs the more will get code like this in our loaders...
(I recall having read some similar parsing code for the cvs one).

I've changed the reviewers to blocking:reviewers so david has the last word on your good adaptations.

Note:
Sorry, it took me some time to actually reading through it (I started in december...).

This revision is now accepted and ready to land.Jan 5 2022, 10:01 AM
ardumont resigned from this revision.Jan 5 2022, 10:01 AM
This revision now requires review to proceed.Jan 5 2022, 10:01 AM
ardumont accepted this revision as: ardumont.Jan 5 2022, 10:02 AM
ardumont removed a reviewer: ardumont. ardumont added 1 blocking reviewer(s): Reviewers.

lather, rinse, repeat (correctly accepting it this time)

anlambert updated this revision to Diff 24955.Jan 5 2022, 4:19 PM

Rebase

swh-public-ci added a comment.Jan 5 2022, 4:20 PM

Build is green

Patch application report for D6839 (id=24955)

Rebasing onto 1b13c5c19e...

Current branch diff-target is up to date.
Changes applied before test
commit aaaa4bc8dc3cb65fdca2220307bbd80f1415f84e
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/222/ for more details.

Harbormaster completed remote builds in B25826: Diff 24955.Jan 5 2022, 4:20 PM
anlambert updated this revision to Diff 25007.Jan 7 2022, 6:53 PM

Fix edge case encountered while testing on real world repositories.

swh-public-ci added a comment.Jan 7 2022, 6:55 PM

Build is green

Patch application report for D6839 (id=25007)

Rebasing onto 1b13c5c19e...

Current branch diff-target is up to date.
Changes applied before test
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/223/ for more details.

Harbormaster completed remote builds in B25877: Diff 25007.Jan 7 2022, 6:55 PM
ardumont accepted this revision.Jan 10 2022, 10:42 AM
This revision is now accepted and ready to land.Jan 10 2022, 10:42 AM
anlambert added a child revision: D6895: ra: Add support for subversion external definitions.Jan 10 2022, 2:19 PM
anlambert updated this revision to Diff 25097.Jan 12 2022, 2:53 PM

Rebase

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

Build is green

Patch application report for D6839 (id=25097)

Rebasing onto 93b4f2fdd8...

Current branch diff-target is up to date.
Changes applied before test
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/229/ for more details.

Harbormaster completed remote builds in B25971: Diff 25097.Jan 12 2022, 2:55 PM
anlambert updated this revision to Diff 25167.Jan 14 2022, 12:01 PM

Add a boolean in the tuple returned by the parse function indicating
if the external URL was defined as relative to the repository one
bu targets a path outside the repository.

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

Build is green

Patch application report for D6839 (id=25167)

Rebasing onto 93b4f2fdd8...

Current branch diff-target is up to date.
Changes applied before test
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/233/ for more details.

Harbormaster completed remote builds in B26031: Diff 25167.Jan 14 2022, 12:03 PM
anlambert updated this revision to Diff 25222.Jan 18 2022, 10:51 AM

Update: Handle a couple of edge cases found when testing with real world repositories.

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

Build is green

Patch application report for D6839 (id=25222)

Rebasing onto 93b4f2fdd8...

Current branch diff-target is up to date.
Changes applied before test
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/245/ for more details.

Harbormaster completed remote builds in B26083: Diff 25222.Jan 18 2022, 10:53 AM
anlambert updated this revision to Diff 25237.Jan 18 2022, 11:09 AM

Rebase

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

Build is green

Patch application report for D6839 (id=25237)

Rebasing onto cb4bf60c0e...

Current branch diff-target is up to date.
Changes applied before test
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/249/ for more details.

Harbormaster completed remote builds in B26098: Diff 25237.Jan 18 2022, 11:11 AM
Closed by commit rDLDSVNf1913512a5fa: utils: Add a function to parse a subversion external definition (authored by anlambert). · Explain WhyJan 18 2022, 12:50 PM
This revision was automatically updated to reflect the committed changes.
anlambert added a commit: rDLDSVNf1913512a5fa: utils: Add a function to parse a subversion external definition.

Revision Contents
Changeset List

PathSize
swh/
loader/
svn/
tests/
261 lines
128 lines

Diff 25252

View Options

swh/loader/svn/tests/test_utils.py

Loading...
View Options

swh/loader/svn/utils.py

Loading...
About · Developer information · Legal