Paths

Table of Contentst

Differential D6690

svn: Use a temporary directory when checking if history got altered
ClosedPublic
Actions

Authored by anlambert on Nov 25 2021, 2:33 PM.

Details

Reviewers

ardumont

Group Reviewers

Reviewers

Maniphest Tasks

T3747: svn: FileExistsError when creating directory

Commits

rDLDSVNed3c0c986bfb: svn: Use a temporary directory when checking if history got altered

Summary

Previously when checking if svn history got altered when performing
incremental load, the last loaded subversion revision was exported
in the same directory than the one used to replay the svn revisions.

As this directory was not cleared between the history check and
the revisions replay, this could cause errors when replaying revisions
and thus make the loading fails.

So ensure to use a new temporary directory when checking altered
history and clear it when that process has been performed.

Also remove now useless reset of ra.Replay object.

Related to T3747

This should fix SWH-LOADER-SVN-3G and SWH-LOADER-SVN-37.

Diff Detail

Repository

rDLDSVN Subversion (SVN) loader

Branch

master

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 25171
Build 39327: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 39326: arc lint + arc unit

Event Timeline

anlambert created this revision.Nov 25 2021, 2:33 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptNov 25 2021, 2:33 PM

Build is green

Patch application report for D6690 (id=24301)

Rebasing onto 36ada262d4...

Current branch diff-target is up to date.

Changes applied before test

commit 4218fa3e3c19948a1ef1c5d047d9ce537383ada0
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Thu Nov 25 14:25:21 2021 +0100

    svn: Use a temporary directory when checking if history got altered
    
    Previously when checking if svn history got altered when performing
    incremental load, the last loaded subversion revision was exported
    in the same directory than the one used to replay the svn revisions.
    
    As this directory was not cleared between the history check and
    the revisions replay, this could cause errors when replaying revisions
    and thus make the loading fails.
    
    So ensure to use a new temporary directory when checking altered
    history and clear it when that process has been performed.
    
    Also remove now useless reset of ra.Replay object.
    
    Related to T3747

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

Harbormaster completed remote builds in B25171: Diff 24301.Nov 25 2021, 2:35 PM

anlambert requested review of this revision.Nov 25 2021, 2:35 PM

ardumont accepted this revision.Nov 25 2021, 3:12 PM

ardumont added a subscriber: ardumont.

ardumont added inline comments.

swh/loader/svn/svn.py
189–198	this becomes dead code with your change. And the coverage report agrees ;)
275
282	I think you can then remove entirely the swh.loader.svn.export method declaration since no one uses it after your change.

This revision is now accepted and ready to land.Nov 25 2021, 3:12 PM

anlambert added inline comments.Nov 25 2021, 3:17 PM

swh/loader/svn/svn.py
282	ack, I noticed it but hesitated to remove it in case we would like to use it again later. Anyway `export_temporary` does the same thing in a more proper way so I guess we can remove `export` after all.

Update: Address @ardumont comments

ardumont added inline comments.Nov 25 2021, 3:20 PM

swh/loader/svn/svn.py
282	Yes, i think that was the same trail of thought which made me define both. Let's use the more clean one if the case arise (as you said export_temporary ;)

Build is green

Patch application report for D6690 (id=24303)

Rebasing onto 36ada262d4...

Current branch diff-target is up to date.

Changes applied before test

commit ed3c0c986bfb6cfaeb8c783eeba3d8b16c4a45b3
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Thu Nov 25 14:25:21 2021 +0100

    svn: Use a temporary directory when checking if history got altered
    
    Previously when checking if svn history got altered when performing
    incremental load, the last loaded subversion revision was exported
    in the same directory than the one used to replay the svn revisions.
    
    As this directory was not cleared between the history check and
    the revisions replay, this could cause errors when replaying revisions
    and thus make the loading fails.
    
    So ensure to use a new temporary directory when checking altered
    history and clear it when that process has been performed.
    
    Also remove now useless reset of ra.Replay object.
    
    Related to T3747

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

Harbormaster completed remote builds in B25173: Diff 24303.Nov 25 2021, 3:22 PM

Closed by commit rDLDSVNed3c0c986bfb: svn: Use a temporary directory when checking if history got altered (authored by anlambert). · Explain WhyNov 25 2021, 3:22 PM

This revision was automatically updated to reflect the committed changes.

anlambert added a commit: rDLDSVNed3c0c986bfb: svn: Use a temporary directory when checking if history got altered.