Skip to content

Commit

Permalink
Allow users to specify build comments (#815)
Browse files Browse the repository at this point in the history
Write some more metadata for packages (augmenting and renaming the
.full-dependencies file into .meta.json), including a user-supplied comment
string, if any.

Users can supply comments using the new `--annotate PACKAGE=COMMENT` flag.

Currently, it is not checked whether the comment is actually applied. If the
package is already installed, its comment is silently ignored.
  • Loading branch information
TimoWilken authored Jan 22, 2024
1 parent f399a98 commit 3154182
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 16 deletions.
4 changes: 2 additions & 2 deletions aliBuild
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def doMain(args, parser):

if args.action == "version":
print("aliBuild version: {version} ({arch})".format(
version=__version__, arch=args.architecture or "unknown"))
version=__version__ or "unknown", arch=args.architecture or "unknown"))
sys.exit(0)

if args.action == "doctor":
Expand Down Expand Up @@ -91,7 +91,7 @@ if __name__ == "__main__":
logger.setLevel(logging.DEBUG if args.debug else logging.INFO)

os.environ["ALIBUILD_ANALYTICS_ID"] = "UA-77346950-1"
os.environ["ALIBUILD_VERSION"] = __version__
os.environ["ALIBUILD_VERSION"] = __version__ or ""
if args.action == "analytics":
if args.state == "off":
disable_analytics()
Expand Down
4 changes: 2 additions & 2 deletions alibuild_helpers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@
try:
from setuptools_scm import get_version
except ImportError:
__version__ = '(could not detect version)'
__version__ = None
else:
import os.path
source_root = os.path.join(os.path.dirname(__file__), os.path.pardir)
try:
__version__ = get_version(root=source_root)
except LookupError:
__version__ = '(could not detect version)'
__version__ = None
finally:
del get_version, source_root
16 changes: 16 additions & 0 deletions alibuild_helpers/args.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,12 @@ def doParseArgs():
"same effect as adding 'force_rebuild: true' to its recipe "
"in CONFIGDIR. You can specify this option multiple times or "
"separate multiple arguments with commas."))
build_parser.add_argument("--annotate", default=[], action="append", metavar="PACKAGE=COMMENT",
help=("Store COMMENT in the build metadata for PACKAGE. This option "
"can be given multiple times, if you want to store comments "
"in multiple packages. The comment will only be stored if "
"PACKAGE is compiled or downloaded during this run; if it "
"already exists, this does not happen."))

build_docker = build_parser.add_argument_group(title="Build inside a container", description="""\
Builds can be done inside a Docker container, to make it easier to get a
Expand Down Expand Up @@ -434,6 +440,16 @@ def finaliseArgs(args, parser):
if args.docker and not args.dockerImage:
args.dockerImage = "registry.cern.ch/alisw/%s-builder" % args.architecture.split("_")[0]

if "annotate" in args:
for comment_assignment in args.annotate:
if "=" not in comment_assignment:
parser.error("--annotate takes arguments of the form PACKAGE=COMMENT")
args.annotate = {
package: comment
for package, _, comment
in (assignment.partition("=") for assignment in args.annotate)
}

if args.action in ("build", "doctor"):
args.configDir = args.configDir

Expand Down
48 changes: 40 additions & 8 deletions alibuild_helpers/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,44 @@ def generate_initdotsh(package, specs, architecture, post_build=False):
return "\n".join(lines)


def create_provenance_info(package, specs, args):
"""Return a metadata record for storage in the package's install directory."""

def spec_info(spec):
return {
"name": spec["package"],
"tag": spec.get("tag"),
"source": spec.get("source"),
"version": spec["version"],
"revision": spec["revision"],
"hash": spec["hash"],
}

def dependency_list(key):
return [spec_info(specs[dep]) for dep in specs[package].get(key, ())]

return json.dumps({
"comment": args.annotate.get(package),
"alibuild_version": __version__,
"alidist": {
"commit": os.environ["ALIBUILD_ALIDIST_HASH"],
},
"architecture": args.architecture,
"defaults": args.defaults,
"package": spec_info(specs[package]),
"dependencies": {
"direct": {
"build": dependency_list("build_requires"),
"runtime": dependency_list("runtime_requires"),
},
"recursive": { # includes direct deps and deps' deps
"build": dependency_list("full_build_requires"),
"runtime": dependency_list("full_runtime_requires"),
},
},
})


def doBuild(args, parser):
if args.remoteStore.startswith("http"):
syncHelper = HttpRemoteSync(args.remoteStore, args.architecture, args.workDir, args.insecure)
Expand Down Expand Up @@ -491,7 +529,7 @@ def doBuild(args, parser):
debug("Building for architecture %s", args.architecture)
debug("Number of parallel builds: %d", args.jobs)
debug("Using aliBuild from alibuild@%s recipes in alidist@%s",
__version__, os.environ["ALIBUILD_ALIDIST_HASH"])
__version__ or "unknown", os.environ["ALIBUILD_ALIDIST_HASH"])

install_wrapper_script("git", workDir)

Expand Down Expand Up @@ -1071,13 +1109,7 @@ def doBuild(args, parser):
dieOnError(err, "Failed to create script dir %s: %s" % (scriptDir, out))
writeAll("%s/%s.sh" % (scriptDir, spec["package"]), spec["recipe"])
writeAll("%s/build.sh" % scriptDir, cmd_raw % {
"dependenciesJSON": json.dumps({dep: {
"architecture": args.architecture,
"package": dep,
"version": specs[dep]["version"],
"revision": specs[dep]["revision"],
"hash": specs[dep]["hash"],
} for dep in spec.get("full_requires", ())}),
"provenance": create_provenance_info(spec["package"], specs, args),
"initdotsh_deps": generate_initdotsh(p, specs, args.architecture, post_build=False),
"initdotsh_full": generate_initdotsh(p, specs, args.architecture, post_build=True),
"develPrefix": develPrefix,
Expand Down
8 changes: 4 additions & 4 deletions alibuild_helpers/build_template.sh
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ fi
mkdir -p "$INSTALLROOT" "$BUILDROOT" "$BUILDDIR" "$WORK_DIR/INSTALLROOT/$PKGHASH/$PKGPATH"

cd "$WORK_DIR/INSTALLROOT/$PKGHASH"
cat <<\EOF > "$INSTALLROOT/.full-dependencies"
%(dependenciesJSON)s
cat > "$INSTALLROOT/.meta.json" <<\EOF
%(provenance)s
EOF

# Add "source" command for dependencies to init.sh.
Expand Down Expand Up @@ -207,8 +207,8 @@ source_up
unset DYLD_LIBRARY_PATH
EOF

cat <<\EOF > "$INSTALLROOT/.full-dependencies"
%(dependenciesJSON)s
cat > "$INSTALLROOT/.meta.json" <<\EOF
%(provenance)s
EOF

cd "$WORK_DIR/INSTALLROOT/$PKGHASH/$PKGPATH"
Expand Down
1 change: 1 addition & 0 deletions tests/test_args.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"build zlib --architecture foo": ARCHITECTURE_ERROR,
"build --force-unknown-architecture zlib --remote-store rsync://test1.local/::rw --write-store rsync://test2.local/::rw ": [call('cannot specify ::rw and --write-store at the same time')],
"build zlib -a osx_x86-64 --docker-image foo": [call('cannot use `-a osx_x86-64` and --docker')],
"build zlib -a slc7_x86-64 --annotate foobar": [call("--annotate takes arguments of the form PACKAGE=COMMENT")],
"analytics": [call(ANALYTICS_MISSING_STATE_ERROR)]
}

Expand Down
1 change: 1 addition & 0 deletions tests/test_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ def test_coverDoBuild(self, mock_debug, mock_listdir, mock_warning, mock_sys, mo
force_rebuild=[],
defaults="release",
jobs=2,
annotate={},
preferSystem=[],
noSystem=False,
debug=True,
Expand Down

0 comments on commit 3154182

Please sign in to comment.