Page MenuHomeSoftware Heritage
Differential D6620

ra: Remove global variables.
ClosedPublic
Actions

Authored by vlorentz on Nov 8 2021, 2:21 PM.

Details

Reviewers
anlambert
ardumont
Group Reviewers
Reviewers
Commits
rDLDSVN65c6ae9e1275: ra: Remove global variables.
Summary

Depends on D6601

There is one change in behavior: svn_special_path_non_link_data is reset at the same time as eol_style for a file. I assume it was an oversight not to do it before this diff

Diff Detail

Repository
rDLDSVN Subversion (SVN) loader
Branch
global-vars
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 24934
Build 38961: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 38960: arc lint + arc unit

Event Timeline

vlorentz created this revision.Nov 8 2021, 2:21 PM
swh-public-ci added a comment.Nov 8 2021, 2:21 PM

Build has FAILED

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

Harbormaster returned this revision to the author for changes because remote builds failed.Nov 8 2021, 2:21 PM
Harbormaster failed remote builds in B24907: Diff 24033!
vlorentz edited the summary of this revision. (Show Details)Nov 8 2021, 2:22 PM
vlorentz updated this revision to Diff 24034.Nov 8 2021, 2:22 PM

rebase

swh-public-ci added a comment.Nov 8 2021, 2:23 PM

Build has FAILED

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

Harbormaster returned this revision to the author for changes because remote builds failed.Nov 8 2021, 2:23 PM
Harbormaster failed remote builds in B24908: Diff 24034!
vlorentz updated this revision to Diff 24045.Nov 8 2021, 4:21 PM

rebase + make the state persist

vlorentz removed a parent revision: D6601: ra: Fix export of non link file with svn:special property set.Nov 8 2021, 4:22 PM
vlorentz added a parent revision: D6601: ra: Fix export of non link file with svn:special property set.
vlorentz edited the summary of this revision. (Show Details)
swh-public-ci added a comment.Nov 8 2021, 4:23 PM

Build is green

Patch application report for D6620 (id=24045)

Rebasing onto 58a07c67d5...

Current branch diff-target is up to date.
Changes applied before test
commit befe78727ec470027c2b3595a456c01757ab3f46
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Nov 8 14:20:44 2021 +0100

    ra: Remove global variables.

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

Harbormaster completed remote builds in B24919: Diff 24045.Nov 8 2021, 4:23 PM
vlorentz requested review of this revision.Nov 8 2021, 4:23 PM
vlorentz updated this revision to Diff 24046.Nov 8 2021, 4:32 PM

delete states recursively again

vlorentz edited the summary of this revision. (Show Details)Nov 8 2021, 4:33 PM
swh-public-ci added a comment.Nov 8 2021, 4:34 PM

Build is green

Patch application report for D6620 (id=24046)

Rebasing onto 58a07c67d5...

Current branch diff-target is up to date.
Changes applied before test
commit 476aa30b230d0cabb8b7f341589769ee60c936d6
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Nov 8 14:20:44 2021 +0100

    ra: Remove global variables.

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

Harbormaster completed remote builds in B24920: Diff 24046.Nov 8 2021, 4:34 PM
ardumont added a subscriber: ardumont.Nov 8 2021, 4:57 PM

ok, thanks.

Can you please add types to the constructor parameter you updated?

swh/loader/svn/ra.py
151

Please add types ;)

350

make it a more sensible debug statement instead?

ardumont accepted this revision.Nov 8 2021, 4:57 PM
This revision is now accepted and ready to land.Nov 8 2021, 4:57 PM
ardumont added a comment.Nov 8 2021, 5:07 PM

There is one change in behavior: svn_special_path_non_link_data is reset at the same time as eol_style for a file. I assume it was an oversight not to do it before this diff

I'm not entirely sure... But tests are ok so let's try it.

vlorentz updated this revision to Diff 24059.Nov 9 2021, 11:38 AM

add typeann + rebase

vlorentz added 1 blocking reviewer(s): anlambert.Nov 9 2021, 11:38 AM

@anlambert Can you have a look too?

This revision now requires review to proceed.Nov 9 2021, 11:38 AM
swh-public-ci added a comment.Nov 9 2021, 11:40 AM

Build is green

Patch application report for D6620 (id=24059)

Rebasing onto 189dfd5300...

Current branch diff-target is up to date.
Changes applied before test
commit 2a259e0d206862aad806a0ce48923d9f94f9aa17
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Nov 8 14:20:44 2021 +0100

    ra: Remove global variables.
    
    There is one change in behavior: svn_special_path_non_link_data is reset
    at the same time as eol_style for a file.
    I assume it was an oversight not to do it before this diff

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

Harbormaster completed remote builds in B24934: Diff 24059.Nov 9 2021, 11:40 AM
anlambert accepted this revision.Nov 15 2021, 1:45 PM

Looks good to me, better indeed.

I don't know why but in my mind the root Editor class was recreated between each revision load, I should have read the code of the Replay class more carefully.

swh/loader/svn/ra.py
129

Could you add a small docstring to explain the purpose of that class ?

350

not sure there is an interest to log such a debug statement here, looks like a debug print that has been accidentally committed.

This revision is now accepted and ready to land.Nov 15 2021, 1:45 PM
vlorentz added inline comments.Nov 15 2021, 3:04 PM
swh/loader/svn/ra.py
129

Any suggestion? I'm not sure I understand it myself...

350

indeed

anlambert added inline comments.Nov 15 2021, 3:15 PM
swh/loader/svn/ra.py
129

Could be something like: Class used to persist some file states (end of lines style for instance) across revisions replay when associated subversion properties are modified.

vlorentz updated this revision to Diff 24134.Nov 15 2021, 3:17 PM

add docstring, remove debug print

swh-public-ci added a comment.Nov 15 2021, 3:19 PM

Build is green

Patch application report for D6620 (id=24134)

Rebasing onto 0f70f07bc3...

First, rewinding head to replay your work on top of it...
Applying: ra: Remove global variables.
Changes applied before test
commit d6259ee542918edcdf6abfdcabeb5bd98846dc11
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Nov 8 14:20:44 2021 +0100

    ra: Remove global variables.
    
    There is one change in behavior: svn_special_path_non_link_data is reset
    at the same time as eol_style for a file.
    I assume it was an oversight not to do it before this diff

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

Harbormaster completed remote builds in B25014: Diff 24134.Nov 15 2021, 3:19 PM
vlorentz updated this revision to Diff 24136.Nov 15 2021, 5:21 PM

rebase

This revision was landed with ongoing or failed builds.Nov 15 2021, 5:21 PM
Closed by commit rDLDSVN65c6ae9e1275: ra: Remove global variables. (authored by vlorentz). · Explain Why
This revision was automatically updated to reflect the committed changes.
vlorentz added a commit: rDLDSVN65c6ae9e1275: ra: Remove global variables..
swh-public-ci added a comment.Nov 15 2021, 5:23 PM

Build is green

Patch application report for D6620 (id=24136)

Rebasing onto 65c6ae9e12...

First, rewinding head to replay your work on top of it...
Fast-forwarded diff-target to base-revision-189-D6620.
Changes applied before test



See https://jenkins.softwareheritage.org/job/DLDSVN/job/tests-on-diff/189/ for more details.
Harbormaster completed remote builds in B25016: Diff 24136.Nov 15 2021, 5:23 PM
Revision Contents
Changeset List
PathSizeCoverage (All)Coverage (Touched)
swh/
loader/
svn/
78 lines86%
Loading...
Diff 24059
View Options
swh/loader/svn/ra.py
Loading...
About · Developer information · Legal