Page MenuHomeSoftware Heritage

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

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

Details

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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

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.

ardumont added a subscriber: ardumont.
ardumont added inline comments.
swh/loader/svn/svn.py
188–189

this becomes dead code with your change.
And the coverage report agrees ;)

271

I think you can then remove entirely the swh.loader.svn.export method declaration since no one uses it after your change.

275
This revision is now accepted and ready to land.Nov 25 2021, 3:12 PM
swh/loader/svn/svn.py
271

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.

swh/loader/svn/svn.py
271

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.