Page MenuHomeSoftware Heritage
Differential D6009

opam: Avoid unnecessary raise instructions
ClosedPublic
Actions

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

Details

Reviewers
aleo
Group Reviewers
Reviewers
Maniphest Tasks
T3425: Opam loader
Commits
rDLDBASE27553732951e: opam: Avoid unnecessary raise instructions
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

Event Timeline

ardumont created this revision.Jul 20 2021, 3:47 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptJul 20 2021, 3:47 PM
ardumont mentioned this in D5975: add opam loader.Jul 20 2021, 3:50 PM
swh-public-ci added a comment.Jul 20 2021, 3:50 PM

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.

Harbormaster completed remote builds in B22670: Diff 21701.Jul 20 2021, 3:50 PM
ardumont requested review of this revision.Jul 20 2021, 3:50 PM
ardumont updated this revision to Diff 21704.Jul 20 2021, 4:05 PM

Rebase and rework commit message

ardumont updated this revision to Diff 21705.Jul 20 2021, 4:05 PM

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)
ardumont added a task: T3425: Opam loader.
swh-public-ci added a comment.Jul 20 2021, 4:07 PM

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.

Harbormaster completed remote builds in B22673: Diff 21704.Jul 20 2021, 4:07 PM
swh-public-ci added a comment.Jul 20 2021, 4:10 PM

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.

Harbormaster completed remote builds in B22674: Diff 21705.Jul 20 2021, 4:10 PM
ardumont added a reviewer: aleo.Jul 20 2021, 4:18 PM
aleo accepted this revision.Jul 20 2021, 4:20 PM

Looks good to me, thanks ! :)

This revision is now accepted and ready to land.Jul 20 2021, 4:20 PM
ardumont updated this revision to Diff 21709.Jul 20 2021, 4:36 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.
swh-public-ci added a comment.Jul 20 2021, 4:38 PM

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.

Harbormaster completed remote builds in B22678: Diff 21709.Jul 20 2021, 4:38 PM
Closed by commit rDLDBASE27553732951e: opam: Avoid unnecessary raise instructions (authored by ardumont). · Explain WhyJul 20 2021, 4:40 PM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDLDBASE27553732951e: opam: Avoid unnecessary raise instructions.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
loader/
package/
opam/
64 lines85%
Loading...

Diff 21705

View Options

swh/loader/package/opam/loader.py

Loading...
About · Developer information · Legal