Differential D6559

cvsclient: handle additional responses sent by server
ClosedPublic
Actions

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

Details

Reviewers

ardumont

Group Reviewers

Reviewers

Maniphest Tasks

T3691: Implement CVS loader

Commits

rDLDCVS6ff0b4473c2b: apply style tweaks suggested by vlorentz and reformatted by black
rDLDCVS3a2f06b3d5fb: cvsclient: handle additional responses sent by server

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 jenkins	Jenkins console · Jenkins
Build 38577: arc lint + arc unit

Event Timeline

stsp created this revision.Oct 27 2021, 12:25 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptOct 27 2021, 12:25 PM

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.

Harbormaster completed remote builds in B24716: Diff 23833.Oct 27 2021, 12:27 PM

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

stsp mentioned this in T3691: Implement CVS loader.Oct 27 2021, 12:36 PM

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

This revision was landed with ongoing or failed builds.Oct 27 2021, 3:52 PM

Closed by commit rDLDCVS3a2f06b3d5fb: cvsclient: handle additional responses sent by server (authored by stsp). · Explain Why

This revision was automatically updated to reflect the committed changes.

stsp added a commit: rDLDCVS3a2f06b3d5fb: cvsclient: handle additional responses sent by server.

stsp added a commit: rDLDCVS6ff0b4473c2b: 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.

Harbormaster completed remote builds in B24732: Diff 23849.Oct 27 2021, 3:53 PM

stsp added a task: T3691: Implement CVS loader.Oct 27 2021, 3:55 PM