Skip to content

Commit 37800f3

Browse files
authored
Fix prefer_system_check failing in aliBuild but not aliDoctor (#880) (#996)
Checks that depended on a variable that had `_VERSION` were silently failing in `aliBuild build` but not in `aliBuild doctor`, causing unnecesary rebuilds Regression from #327
1 parent 104f886 commit 37800f3

File tree

3 files changed

+45
-3
lines changed

3 files changed

+45
-3
lines changed

alibuild_helpers/build.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from alibuild_helpers.log import debug, info, banner, warning
77
from alibuild_helpers.log import dieOnError
88
from alibuild_helpers.cmd import execute, DockerRunner, BASH, install_wrapper_script, getstatusoutput
9-
from alibuild_helpers.utilities import prunePaths, symlink, call_ignoring_oserrors, topological_sort, detectArch
9+
from alibuild_helpers.utilities import pruneWorkdirFromPaths, pruneVersionEnvVars, symlink, call_ignoring_oserrors, topological_sort, detectArch
1010
from alibuild_helpers.utilities import resolve_store_path
1111
from alibuild_helpers.utilities import parseDefaults, readDefaults
1212
from alibuild_helpers.utilities import getPackageList, asList
@@ -460,7 +460,7 @@ def doBuild(args, parser):
460460
specs = {}
461461
buildOrder = []
462462
workDir = abspath(args.workDir)
463-
prunePaths(workDir)
463+
pruneWorkdirFromPaths(workDir)
464464

465465
dieOnError(not exists(args.configDir),
466466
'Cannot find alidist recipes under directory "%s".\n'
@@ -534,6 +534,8 @@ def performPreferCheckWithTempDir(pkg, cmd):
534534
taps = taps,
535535
log = debug)
536536

537+
pruneVersionEnvVars()
538+
537539
dieOnError(validDefaults and args.defaults not in validDefaults,
538540
"Specified default `%s' is not compatible with the packages you want to build.\n"
539541
"Valid defaults:\n\n- %s" % (args.defaults, "\n- ".join(sorted(validDefaults or []))))

alibuild_helpers/utilities.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,16 +169,26 @@ def resolve_tag(spec):
169169
def normalise_multiple_options(option, sep=","):
170170
return [x for x in ",".join(option).split(sep) if x]
171171

172-
def prunePaths(workDir):
172+
def pruneWorkdirFromPaths(workDir):
173+
"""Remove workDir entries from PATH, LD_LIBRARY_PATH, and DYLD_LIBRARY_PATH."""
173174
for x in ["PATH", "LD_LIBRARY_PATH", "DYLD_LIBRARY_PATH"]:
174175
if x not in os.environ:
175176
continue
176177
workDirEscaped = re.escape("%s" % workDir) + "[^:]*:?"
177178
os.environ[x] = re.sub(workDirEscaped, "", os.environ[x])
179+
180+
181+
def pruneVersionEnvVars():
182+
"""Remove all *_VERSION environment variables except ALIBUILD_VERSION."""
178183
for x in list(os.environ.keys()):
179184
if x.endswith("_VERSION") and x != "ALIBUILD_VERSION":
180185
os.environ.pop(x)
181186

187+
188+
def prunePaths(workDir):
189+
pruneWorkdirFromPaths(workDir)
190+
pruneVersionEnvVars()
191+
182192
def validateSpec(spec):
183193
if not spec:
184194
raise SpecError("Empty recipe.")

tests/test_packagelist.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,16 @@
7878
exit 0
7979
---
8080
"""),
81+
# Recipe for issue #880: prefer_system_check relies on _VERSION env var
82+
"CONFIG_DIR/version-check.sh": dedent("""\
83+
package: version-check
84+
version: v1
85+
prefer_system: '.*'
86+
prefer_system_check: |
87+
[ "$GENFIT_VERSION" = "v1.0.0" ] && exit 0
88+
exit 1
89+
---
90+
"""),
8191
}
8292

8393
class MockReader:
@@ -229,6 +239,26 @@ def fake_exists(n):
229239
self.assertTrue(specs["defaults-release"]["force_rebuild"])
230240

231241

242+
@mock.patch("alibuild_helpers.utilities.getRecipeReader", new=MockReader)
243+
class Issue880TestCase(unittest.TestCase):
244+
"""Test for issue #880: pruneWorkdirFromPaths preserves _VERSION env vars."""
245+
246+
def test_version_check_uses_env_var(self) -> None:
247+
"""prefer_system_check can use _VERSION env vars after pruneWorkdirFromPaths."""
248+
from alibuild_helpers.utilities import pruneWorkdirFromPaths
249+
250+
def fake_exists(n):
251+
return n in RECIPES.keys()
252+
253+
os.environ["GENFIT_VERSION"] = "v1.0.0"
254+
try:
255+
with patch.object(os.path, "exists", fake_exists):
256+
pruneWorkdirFromPaths("/fake/workdir")
257+
self.assertIn("GENFIT_VERSION", os.environ)
258+
_, systemPkgs, ownPkgs, _, _ = getPackageListWithDefaults(["version-check"])
259+
self.assertIn("version-check", systemPkgs)
260+
finally:
261+
os.environ.pop("GENFIT_VERSION", None)
232262

233263

234264
if __name__ == '__main__':

0 commit comments

Comments
 (0)