Page MenuHomeSoftware Heritage

Use minimal environment in Mercurial loader
ClosedPublic

Authored by Alphare on Feb 25 2021, 3:56 PM.

Details

Summary

This generalizes the work done in ef3a2ba79ea9 to (supposedly) all
places invoking Mercurial. In short, this limits the environment to the
smallest subset needed (i.e. $PATH) and uses the Mercurial-specific
variables to disable user customizations and configs.

Diff Detail

Repository
rDLDHG Mercurial loader
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Build has FAILED

Patch application report for D5146 (id=18406)

Rebasing onto d8572187e9...

Current branch diff-target is up to date.
Changes applied before test
commit 99df13c6cb912cb272c5a84c8ad6cfd79576dfb5
Author: Raphaël Gomès <rgomes@octobus.net>
Date:   Thu Feb 25 15:51:24 2021 +0100

    Use minimal environment in Mercurial loader
    
    This generalizes the work done in ef3a2ba79ea9 to (supposedly) all
    places invoking Mercurial. In short, this limits the environment to the
    smallest subset needed (i.e. `$PATH`) and uses the Mercurial-specific
    variables to disable user customizations and configs.

Link to build: https://jenkins.softwareheritage.org/job/DLDHG/job/tests-on-diff/165/
See console output for more information: https://jenkins.softwareheritage.org/job/DLDHG/job/tests-on-diff/165/console

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 25 2021, 3:58 PM
Harbormaster failed remote builds in B19484: Diff 18406!

Build is green

Patch application report for D5146 (id=18408)

Rebasing onto d8572187e9...

Current branch diff-target is up to date.
Changes applied before test
commit c7999db35929e599a0ecff5eb77a16d38818a8d7
Author: Raphaël Gomès <rgomes@octobus.net>
Date:   Thu Feb 25 15:51:24 2021 +0100

    Use minimal environment in Mercurial loader
    
    This generalizes the work done in ef3a2ba79ea9 to (supposedly) all
    places invoking Mercurial. In short, this limits the environment to the
    smallest subset needed (i.e. `$PATH`) and uses the Mercurial-specific
    variables to disable user customizations and configs.

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

This revision is now accepted and ready to land.Feb 26 2021, 8:54 AM
vlorentz requested changes to this revision.EditedFeb 26 2021, 8:55 AM

hmm actually, modifying this global may interact with other parts of the code in surprising way.

Could you only use the env argument of subprocess functions instead of changing os.environ?

This revision now requires changes to proceed.Feb 26 2021, 8:55 AM

hmm actually, modifying this global may interact with other parts of the code in surprising way.

Could you only use the env argument of subprocess functions instead of changing os.environ?

That was my thinking also, but it looks like billiard.Process is just Python's Thread in disguise and neither it nor Queue provide a way of setting the environment. Maybe I'm missing something, I'd be glad not to touch global state!

Build is green

Patch application report for D5146 (id=18431)

Rebasing onto 1dbb8474ad...

Current branch diff-target is up to date.
Changes applied before test
commit 30d0c36d0ac84b93aad591d06f6c01b721941ee1
Author: Raphaël Gomès <rgomes@octobus.net>
Date:   Thu Feb 25 15:51:24 2021 +0100

    Use minimal environment in Mercurial loader
    
    This generalizes the work done in ef3a2ba79ea9 to (supposedly) all
    places invoking Mercurial. In short, this limits the environment to the
    smallest subset needed (i.e. `$PATH`) and uses the Mercurial-specific
    variables to disable user customizations and configs.

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

Build is green

Patch application report for D5146 (id=18437)

Rebasing onto 04a1213f36...

Current branch diff-target is up to date.
Changes applied before test
commit c05d78bef086325a5697be926dc108a6c975f9f2
Author: Raphaël Gomès <rgomes@octobus.net>
Date:   Thu Feb 25 15:51:24 2021 +0100

    Use minimal environment in Mercurial loader
    
    This generalizes the work done in ef3a2ba79ea9 to (supposedly) all
    places invoking Mercurial. In short, this limits the environment to the
    smallest subset needed (i.e. `$PATH`) and uses the Mercurial-specific
    variables to disable user customizations and configs.

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

This revision is now accepted and ready to land.Feb 26 2021, 4:40 PM

meh

Agreed

I don't have anything better to propose.

Still, it's sad because i don't like those kind of side effects
in the constructor (env variables manipulation).

¯\_(ツ)_/¯

Build is green

Patch application report for D5146 (id=18456)

Rebasing onto 5b8da68a92...

Current branch diff-target is up to date.
Changes applied before test
commit c74e5791b93ef92ac3688c688de3b28f8e18a2d0
Author: Raphaël Gomès <rgomes@octobus.net>
Date:   Thu Feb 25 15:51:24 2021 +0100

    Use minimal environment in Mercurial loader
    
    This generalizes the work done in ef3a2ba79ea9 to (supposedly) all
    places invoking Mercurial. In short, this limits the environment to the
    smallest subset needed (i.e. `$PATH`) and uses the Mercurial-specific
    variables to disable user customizations and configs.

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