Page MenuHomeSoftware Heritage

opam: Avoid unnecessary raise instructions
ClosedPublic

Authored by ardumont on Jul 20 2021, 3:47 PM.

Details

Summary

This simplifies the logic behind the parsing of the opam_read function to avoid
raising outside of the opam_read call.

Related to T3425

Test Plan

tox

Diff Detail

Repository
rDLDBASE Generic VCS/Package Loader
Branch
arcpatch-D5975
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 22670
Build 35351: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 35350: arc lint + arc unit

Event Timeline

Build is green

Patch application report for D6009 (id=21701)

Could not rebase; Attempt merge onto dbb18628f1...

Auto-merging setup.py
Merge made by the 'recursive' strategy.
 CONTRIBUTORS                                       |   3 +-
 setup.py                                           |   1 +
 swh/loader/package/opam/__init__.py                |  17 ++
 swh/loader/package/opam/loader.py                  | 211 +++++++++++++++++++++
 swh/loader/package/opam/tasks.py                   |  20 ++
 swh/loader/package/opam/tests/__init__.py          |   0
 .../fake_opam_repo/packages/agrid/agrid.0.1/opam   |  38 ++++
 .../packages/directories/directories.0.1/opam      |  38 ++++
 .../packages/directories/directories.0.2/opam      |  38 ++++
 .../packages/directories/directories.0.3/opam      |  43 +++++
 .../data/fake_opam_repo/packages/ocb/ocb.0.1/opam  |  37 ++++
 .../package/opam/tests/data/fake_opam_repo/repo    |   1 +
 .../package/opam/tests/data/fake_opam_repo/version |   1 +
 .../OCamlPro_agrid_archive_0.1.tar.gz              | Bin 0 -> 41584 bytes
 .../OCamlPro_directories_archive_0.1.tar.gz        | Bin 0 -> 8591 bytes
 .../OCamlPro_directories_archive_0.2.tar.gz        | Bin 0 -> 8479 bytes
 .../OCamlPro_directories_archive_0.3.tar.gz        | Bin 0 -> 9619 bytes
 .../OCamlPro_ocb_archive_0.1.tar.gz                | Bin 0 -> 48551 bytes
 swh/loader/package/opam/tests/test_opam.py         | 176 +++++++++++++++++
 swh/loader/package/opam/tests/test_tasks.py        |  27 +++
 20 files changed, 650 insertions(+), 1 deletion(-)
 create mode 100644 swh/loader/package/opam/__init__.py
 create mode 100644 swh/loader/package/opam/loader.py
 create mode 100644 swh/loader/package/opam/tasks.py
 create mode 100644 swh/loader/package/opam/tests/__init__.py
 create mode 100644 swh/loader/package/opam/tests/data/fake_opam_repo/packages/agrid/agrid.0.1/opam
 create mode 100644 swh/loader/package/opam/tests/data/fake_opam_repo/packages/directories/directories.0.1/opam
 create mode 100644 swh/loader/package/opam/tests/data/fake_opam_repo/packages/directories/directories.0.2/opam
 create mode 100644 swh/loader/package/opam/tests/data/fake_opam_repo/packages/directories/directories.0.3/opam
 create mode 100644 swh/loader/package/opam/tests/data/fake_opam_repo/packages/ocb/ocb.0.1/opam
 create mode 100644 swh/loader/package/opam/tests/data/fake_opam_repo/repo
 create mode 100644 swh/loader/package/opam/tests/data/fake_opam_repo/version
 create mode 100644 swh/loader/package/opam/tests/data/https_github.com/OCamlPro_agrid_archive_0.1.tar.gz
 create mode 100644 swh/loader/package/opam/tests/data/https_github.com/OCamlPro_directories_archive_0.1.tar.gz
 create mode 100644 swh/loader/package/opam/tests/data/https_github.com/OCamlPro_directories_archive_0.2.tar.gz
 create mode 100644 swh/loader/package/opam/tests/data/https_github.com/OCamlPro_directories_archive_0.3.tar.gz
 create mode 100644 swh/loader/package/opam/tests/data/https_github.com/OCamlPro_ocb_archive_0.1.tar.gz
 create mode 100644 swh/loader/package/opam/tests/test_opam.py
 create mode 100644 swh/loader/package/opam/tests/test_tasks.py
Changes applied before test
commit 8a7a40274357ab0807f22f4ab5b0fe48cfd001bf
Merge: dbb1862 f6425d6
Author: Jenkins user <jenkins@localhost>
Date:   Tue Jul 20 13:47:50 2021 +0000

    Merge branch 'diff-target' into HEAD

commit f6425d666176360acb9c66e02b95d9b740e229e9
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Jul 20 15:47:10 2021 +0200

    opam: Avoid raising too much
    
    Depends on D5975

commit 5f837ef025d21211fad85170cc8436f92be181d2
Author: zapashcanon <leo@ndrs.fr>
Date:   Tue Jul 20 15:10:07 2021 +0200

    add opam loader
    
    Summary:
    added an opam loader
    
    Related to T3425
    
    Test Plan: will add tests later using a local opam repository as it's done in the lister
    
    Reviewers: #reviewers, vlorentz, ardumont
    
    Reviewed By: #reviewers, ardumont
    
    Subscribers: ardumont, vlorentz
    
    Maniphest Tasks: T3425
    
    Differential Revision: https://forge.softwareheritage.org/D5975

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

Rebase and rework commit message

again commit message rework

ardumont retitled this revision from opam: Avoid raising too much to opam: Avoid unnecessary raise instructions.Jul 20 2021, 4:06 PM
ardumont edited the summary of this revision. (Show Details)

Build is green

Patch application report for D6009 (id=21704)

Rebasing onto 1602e9913b...

Current branch diff-target is up to date.
Changes applied before test
commit 3bd41279ea9378b4a86e56e8d76bc56e2c96fe18
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Jul 20 15:47:10 2021 +0200

    opam: Avoid unnecessary raise instruction
    
    This simplifies the logic behind the parsing of the `opam_read` function to avoid
    raising outside of the `opam_read` call.
    
    Related to T3425

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

Build is green

Patch application report for D6009 (id=21705)

Rebasing onto 1602e9913b...

Current branch diff-target is up to date.
Changes applied before test
commit 8415767790711235d14e9510d52c408a86edf0c0
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Jul 20 15:47:10 2021 +0200

    opam: Avoid unnecessary raise instructions
    
    This simplifies the logic behind the parsing of the `opam_read` function to avoid
    raising outside of the `opam_read` call.
    
    Related to T3425

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

Looks good to me, thanks ! :)

This revision is now accepted and ready to land.Jul 20 2021, 4:20 PM
ardumont edited the summary of this revision. (Show Details)
  • rework comment
  • adding tests around the missing conditionals about Popen etc... is actually not that simple afaict, it will be dealt with at another time.

Build is green

Patch application report for D6009 (id=21709)

Rebasing onto 1602e9913b...

Current branch diff-target is up to date.
Changes applied before test
commit 27553732951e7d18a2f99185da4ee2fa15861e89
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Jul 20 15:47:10 2021 +0200

    opam: Avoid unnecessary raise instructions
    
    This simplifies the logic behind the parsing of the `opam_read` function to avoid
    raising outside of the `opam_read` call.
    
    Related to T3425

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