Page MenuHomeSoftware Heritage

opam: Allow defining where to actually install the opam_root folder
ClosedPublic

Authored by ardumont on Sep 20 2021, 3:10 PM.

Diff Detail

Repository
rDLS Listers
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 D6306 (id=22895)

Rebasing onto c803fc2b59...

Current branch diff-target is up to date.
Changes applied before test
commit ca5a25ec0a16c375cd5c13be77e125eb6311c4a7
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 15:08:58 2021 +0200

    opam: Allow defining where to actually install the opam_root folder
    
    Related to T3590

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

douardda added a subscriber: douardda.

LGTM, but how is the new opam_root option expected to be set (in production I mean)?

This revision is now accepted and ready to land.Sep 20 2021, 4:46 PM

LGTM, but how is the new opam_root option expected to be set (in production I mean)?

It's not expected to be set. Currently, we could not. Our current way of instantiating
the listers would make this break (because other listers which share the configuration
would not know what to do about that opam_root key).

So, it's expected to use that new fallback value.


Since I opened that diff, I'm inclined to make that opam root folder configuration also
depend on the instance_url as well.

Those opam lister/loader are designed to be multi instances. Even though, we have only
one instance "for now". If so, that means, this should be computed within the
constructor.

Something like:

self.opamroot = str(opam_root / self.instance)

(And then adapt the other puppet diff).

  • Adapt to be multi instance aware
  • Rebase on top of other diffs

Build is green

Patch application report for D6306 (id=22905)

Could not rebase; Attempt merge onto c803fc2b59...

Updating c803fc2..7996d10
Fast-forward
 swh/lister/opam/lister.py            | 18 ++++++++----
 swh/lister/opam/tests/test_lister.py | 53 ++++++++++++++++++++++++++++++------
 swh/lister/pattern.py                | 13 +++++++++
 3 files changed, 70 insertions(+), 14 deletions(-)
Changes applied before test
commit 7996d102787950433eaee7ad986c5ddcdf0dcbc0
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 15:08:58 2021 +0200

    opam: Allow defining where to actually install the opam_root folder
    
    Related to T3590

commit 0feddfaced6df52b06a145c1e9c905becf61b4d3
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 17:47:29 2021 +0200

    opam: Make the instance optional and derived from the url
    
    This matches how it's done for all other multi instances listers.
    
    Related to T3590

commit 2d0099bf7206347298e38603130445e79a9bf1dd
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 17:41:19 2021 +0200

    opam: Separate side-effects call from the constructor
    
    We should avoid side-effects in the constructor as much as possible. That avoids
    surprising behavior at object instantiation time. The state if needed should be now
    built within the `swh.lister.patter.Lister.prepare` method.
    
    This also fixes the current test that actually bootstrap a real opam local "clone".
    
    Related to T3590

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

Build is green

Patch application report for D6306 (id=22914)

Could not rebase; Attempt merge onto c803fc2b59...

Updating c803fc2..be96c38
Fast-forward
 swh/lister/opam/lister.py            | 20 ++++++++------
 swh/lister/opam/tests/test_lister.py | 53 ++++++++++++++++++++++++++++++------
 swh/lister/pattern.py                | 13 +++++++++
 3 files changed, 70 insertions(+), 16 deletions(-)
Changes applied before test
commit be96c385c24612ee265df42af71e3da09ca99393
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 15:08:58 2021 +0200

    opam: Allow defining where to actually install the opam_root folder
    
    Related to T3590

commit 34880c73a2154a24a6fd3e0f8b707b951f810ec8
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 17:47:29 2021 +0200

    opam: Make the instance optional and derived from the url
    
    This matches how it's done for all other multi instances listers.
    
    Related to T3590

commit 0af407999ab4932e4dccf51897e0402d748a3f03
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 17:41:19 2021 +0200

    opam: Move the state initialization into the get_pages method
    
    We should avoid side-effects in the constructor as much as possible. That avoids
    surprising behavior at object instantiation time. The state if needed must be
    initialized into the `swh.lister.pattern.Lister.get_pages` method, as preconized in the
    class docstring.
    
    This also fixes the current test that actually bootstrap a real opam local "clone" in
    /tmp.
    
    Related to T3590

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

Build is green

Patch application report for D6306 (id=22918)

Could not rebase; Attempt merge onto c803fc2b59...

Updating c803fc2..50700fd
Fast-forward
 swh/lister/opam/lister.py            | 20 +++++++------
 swh/lister/opam/tests/test_lister.py | 54 ++++++++++++++++++++++++++++++------
 2 files changed, 58 insertions(+), 16 deletions(-)
Changes applied before test
commit 50700fd47379c05b440ad6e97a905ec265a08b8d
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 15:08:58 2021 +0200

    opam: Allow defining where to actually install the opam_root folder
    
    Related to T3590

commit 2398ca85c8c4685465945c305a67279a67f7ba55
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 17:47:29 2021 +0200

    opam: Make the instance optional and derived from the url
    
    This matches how it's done for all other multi instances listers.
    
    Related to T3590

commit c160376b17175bd8fa73adddf2e14b8dcfad947b
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 17:41:19 2021 +0200

    opam: Move the state initialization into the get_pages method
    
    We should avoid side-effects in the constructor as much as possible. That avoids
    surprising behavior at object instantiation time. The state if needed must be
    initialized into the `swh.lister.pattern.Lister.get_pages` method, as preconized in the
    class docstring.
    
    This also fixes the current test that actually bootstrap a real opam local "clone" in
    /tmp.
    
    Related to T3590

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

Build is green

Patch application report for D6306 (id=22924)

Could not rebase; Attempt merge onto c803fc2b59...

Updating c803fc2..332ed8e
Fast-forward
 swh/lister/opam/lister.py            | 20 +++++++------
 swh/lister/opam/tests/test_lister.py | 54 ++++++++++++++++++++++++++++++------
 2 files changed, 58 insertions(+), 16 deletions(-)
Changes applied before test
commit 332ed8e54309985e35acb1e7023a7a2ca6e9e88c
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 15:08:58 2021 +0200

    opam: Allow defining where to actually install the opam_root folder
    
    Related to T3590

commit ff5e86ff489eb1d3c504fe6f0a3392db4d8b6c19
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 17:47:29 2021 +0200

    opam: Make the instance optional and derived from the url
    
    This matches how it's done for all other multi instances listers.
    
    Related to T3590

commit b69b0b7fd6745cd579acf2e5544c3810217bb542
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Sep 20 17:41:19 2021 +0200

    opam: Move the state initialization into the get_pages method
    
    We should avoid side-effects in the constructor as much as possible. That avoids
    surprising behavior at object instantiation time. The state if needed must be
    initialized into the `swh.lister.pattern.Lister.get_pages` method, as preconized in the
    class docstring.
    
    This also fixes the current test that actually bootstrap a real opam local "clone" in
    /tmp.
    
    Related to T3590

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