Page MenuHomeSoftware Heritage

cvsclient: handle additional responses sent by server
ClosedPublic

Authored by stsp on Oct 27 2021, 12:25 PM.

Details

Summary

While checking out files the server sends messages to the CVS
client which provide information about the state of file paths.

Our custom CVS client implementation needs to recognize a few
additional responses the server may send while checking out a
different version of a file which was already checked earlier.
Otherwise our client will error out. We can simply ignore the
messages (and its 2 paths arguments separated by \n) because
we do not manage an actual CVS working copy.

Found while testing ingestion of the GNU dino repository at
cvs.savannah.gnu.org/sources/dino

Diff Detail

Repository
rDLDCVS CVS Loader
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 24716
Build 38578: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 38577: arc lint + arc unit

Event Timeline

Build is green

Patch application report for D6559 (id=23833)

Rebasing onto 7f761b8550...

Current branch diff-target is up to date.
Changes applied before test
commit 7eaef259b073ec0ee0e6d70ae0f7a36cc614b143
Author: Stefan Sperling <stsp@stsp.name>
Date:   Wed Oct 27 12:07:32 2021 +0200

    cvsclient: handle additional responses sent by server
    
    While checking out files the server sends messages to the CVS
    client which provide information about the state of file paths.
    
    Our custom CVS client implementation needs to recognize a few
    additional responses the server may send while checking out a
    different version of a file which was already checked earlier.
    Otherwise our client will error out. We can simply ignore the
    messages (and its 2 paths arguments separated by \n) because
    we do not manage an actual CVS working copy.
    
    Found while testing ingestion of the GNU dino repository at
    cvs.savannah.gnu.org/sources/dino

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

stsp requested review of this revision.Oct 27 2021, 12:27 PM

Looks good. Just a couple suggestions below to simplify the code

swh/loader/cvs/cvsclient.py
365–370

btw

373–387
ardumont added a subscriber: ardumont.

lgtm

I do prefer with val's suggested improvments.

This revision is now accepted and ready to land.Oct 27 2021, 2:26 PM
  • cvsclient: handle additional responses sent by server
  • apply style tweaks suggested by vlorentz and reformatted by black

Build is green

Patch application report for D6559 (id=23849)

Rebasing onto d3b3344bc2...

Current branch diff-target is up to date.
Changes applied before test
commit 6ff0b4473c2bdf2feaee0d4df9b03a6787cd23e1
Author: Stefan Sperling <stsp@stsp.name>
Date:   Wed Oct 27 15:50:03 2021 +0200

    apply style tweaks suggested by vlorentz and reformatted by black

commit 3a2f06b3d5fb45ceab1a8a4721a0b7102e633d24
Author: Stefan Sperling <stsp@stsp.name>
Date:   Wed Oct 27 12:07:32 2021 +0200

    cvsclient: handle additional responses sent by server
    
    While checking out files the server sends messages to the CVS
    client which provide information about the state of file paths.
    
    Our custom CVS client implementation needs to recognize a few
    additional responses the server may send while checking out a
    different version of a file which was already checked earlier.
    Otherwise our client will error out. We can simply ignore the
    messages (and its 2 paths arguments separated by \n) because
    we do not manage an actual CVS working copy.
    
    Found while testing ingestion of the GNU dino repository at
    cvs.savannah.gnu.org/sources/dino

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