From 72a061c8c1ecb8f4b8926b48d794691c5646adb4 Mon Sep 17 00:00:00 2001 From: Max Beutelspacher Date: Mon, 3 Feb 2025 16:17:55 +0100 Subject: [PATCH 1/7] feat: option for package local hash --- README.md | 15 ++++++++++---- ros2nix/ros2nix.py | 51 ++++++++++++++++++++++++++++------------------ 2 files changed, 42 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index 94af0a0..2df8b7f 100644 --- a/README.md +++ b/README.md @@ -113,10 +113,10 @@ the [Autoware][] project as an example. ``` usage: ros2nix [-h] [--output OUTPUT | --output-as-ros-pkg-name | --output-as-nix-pkg-name] - [--output-dir OUTPUT_DIR] [--fetch] [--patches | --no-patches] - [--distro DISTRO] [--src-param SRC_PARAM] - [--source-root SOURCE_ROOT] [--do-check] - [--extra-build-inputs DEP1,DEP2,...] + [--output-dir OUTPUT_DIR] [--fetch] [--use-package-git-hash] + [--patches | --no-patches] [--distro DISTRO] + [--src-param SRC_PARAM] [--source-root SOURCE_ROOT] + [--do-check] [--extra-build-inputs DEP1,DEP2,...] [--extra-propagated-build-inputs DEP1,DEP2,...] [--extra-check-inputs DEP1,DEP2,...] [--extra-native-build-inputs DEP1,DEP2,...] [--flake] @@ -149,6 +149,13 @@ options: determined from the local git work tree. sourceRoot attribute is set if needed and not overridden by --source-root. (default: False) + --use-package-git-hash + When using --fetch, use the git hash of the package + sub-directory instead of the one of the upstream + repo.This will lead to longer generation time and + multiple source checkouts when building but will safe + rebuilds of packages that have not changed. (default: + False) --patches, --no-patches Add local git commits not present in git remote named "origin" to patches in the generated Nix expression. diff --git a/ros2nix/ros2nix.py b/ros2nix/ros2nix.py index c352532..557bffe 100755 --- a/ros2nix/ros2nix.py +++ b/ros2nix/ros2nix.py @@ -16,7 +16,7 @@ import subprocess import sys from contextlib import contextmanager from textwrap import dedent, indent -from typing import Iterable, Set +from typing import Iterable, Set, List from catkin_pkg.package import Package, parse_package_string from superflore.exceptions import UnresolvedDependency @@ -247,6 +247,13 @@ def ros2nix(args): "The fetch function and its parameters are determined from the local git work tree. " "sourceRoot attribute is set if needed and not overridden by --source-root.", ) + + parser.add_argument( + "--use-package-git-hash", + action="store_true", + help="When using --fetch, use the git hash of the package sub-directory instead of the one of the upstream repo." + "This will lead to longer generation time and multiple source checkouts when building but will safe rebuilds of packages that have not changed." + ) parser.add_argument( "--patches", action=argparse.BooleanOptionalAction, @@ -390,40 +397,44 @@ def ros2nix(args): kwargs["src_expr"] = args.src_param elif args.fetch: srcdir = os.path.dirname(source) or "." - url = subprocess.check_output( - "git config remote.origin.url".split(), cwd=srcdir - ).decode().strip() - prefix = subprocess.check_output( - "git rev-parse --show-prefix".split(), cwd=srcdir - ).decode().strip() + def check_output(cmd: List[str]): + return subprocess.check_output(cmd, cwd=srcdir).decode().strip() - toplevel = subprocess.check_output( - "git rev-parse --show-toplevel".split(), cwd=srcdir - ).decode().strip() + url = check_output("git config remote.origin.url".split()) - head = subprocess.check_output( - "git rev-parse HEAD".split(), cwd=srcdir - ).decode().strip() + prefix = check_output("git rev-parse --show-prefix".split()) - if toplevel in git_cache: - info = git_cache[toplevel] + toplevel = check_output("git rev-parse --show-toplevel".split()) + + head = check_output("git rev-parse HEAD".split()) + + def merge_base_to_upstream(commit: str)->str: + return subprocess.check_output(f"git merge-base {head} $(git for-each-ref refs/remotes/origin --format='%(objectname)')", cwd=srcdir,shell=True).decode().strip() + + if args.use_package_git_hash: + # we need to get merge_base again to filter out applied patches from the package git hash + merge_base = merge_base_to_upstream(head) + head = check_output(f"git rev-list {merge_base} -1 -- .".split()) + + key = toplevel + if args.use_package_git_hash: + key= f"{toplevel}/{head}" + if key in git_cache: + info = git_cache[key] upstream_rev = info["rev"] else: # Latest commit present in the upstream repo. If # the local repository doesn't have additional # commits, it is the same as HEAD. Should work # even with detached HEAD. - upstream_rev = subprocess.check_output( - "git merge-base HEAD $(git for-each-ref refs/remotes/origin --format='%(objectname)')", - shell=True, cwd=srcdir - ).decode().strip() + upstream_rev = merge_base_to_upstream(head) info = json.loads( subprocess.check_output( ["nix-prefetch-git", "--quiet", toplevel, upstream_rev], ).decode() ) - git_cache[toplevel] = info + git_cache[key] = info match = re.match("https://github.com/(?P[^/]*)/(?P.*?)(.git|/.*)?$", url) if match is not None: From 2b569c35f093bd7f9e1af60221b75bf6869368d2 Mon Sep 17 00:00:00 2001 From: Max Beutelspacher Date: Wed, 5 Feb 2025 18:47:04 +0100 Subject: [PATCH 2/7] feat: use sparse checkout if --use-package-git-hash is provided --- ros2nix/ros2nix.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/ros2nix/ros2nix.py b/ros2nix/ros2nix.py index 557bffe..dcd18b6 100755 --- a/ros2nix/ros2nix.py +++ b/ros2nix/ros2nix.py @@ -417,11 +417,8 @@ def ros2nix(args): merge_base = merge_base_to_upstream(head) head = check_output(f"git rev-list {merge_base} -1 -- .".split()) - key = toplevel - if args.use_package_git_hash: - key= f"{toplevel}/{head}" - if key in git_cache: - info = git_cache[key] + if not args.use_package_git_hash and toplevel in git_cache: #only use cache if not using seperate checkout per package + info = git_cache[toplevel] upstream_rev = info["rev"] else: # Latest commit present in the upstream repo. If @@ -431,12 +428,13 @@ def ros2nix(args): upstream_rev = merge_base_to_upstream(head) info = json.loads( subprocess.check_output( - ["nix-prefetch-git", "--quiet", toplevel, upstream_rev], + ["nix-prefetch-git", "--quiet"]+ (["--sparse-checkout", prefix] if (prefix and args.use_package_git_hash) else [])+[ toplevel, upstream_rev], ).decode() ) - git_cache[key] = info + git_cache[toplevel] = info match = re.match("https://github.com/(?P[^/]*)/(?P.*?)(.git|/.*)?$", url) + sparse_checkout = f"sparseCheckout = [\"{prefix}\"];" if (prefix and args.use_package_git_hash) else "" if match is not None: kwargs["src_param"] = "fetchFromGitHub" kwargs["src_expr"] = dedent(f''' @@ -444,7 +442,8 @@ def ros2nix(args): owner = "{match["owner"]}"; repo = "{match["repo"]}"; rev = "{info["rev"]}"; - sha256 = "{info["sha256"]}"; + sha256 = "{info["sha256"]}"; + {sparse_checkout} }}''').strip() else: kwargs["src_param"] = "fetchgit" @@ -453,6 +452,7 @@ def ros2nix(args): url = "{url}"; rev = "{info["rev"]}"; sha256 = "{info["sha256"]}"; + {sparse_checkout} }}''').strip() if prefix: From e523ecb7f583364b7d77cfe7ef7d4b4780fdd176 Mon Sep 17 00:00:00 2001 From: Michal Sojka Date: Fri, 7 Feb 2025 15:17:19 +0100 Subject: [PATCH 3/7] Fix formatting and typos --- ros2nix/ros2nix.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/ros2nix/ros2nix.py b/ros2nix/ros2nix.py index dcd18b6..6b7345e 100755 --- a/ros2nix/ros2nix.py +++ b/ros2nix/ros2nix.py @@ -402,14 +402,11 @@ def ros2nix(args): return subprocess.check_output(cmd, cwd=srcdir).decode().strip() url = check_output("git config remote.origin.url".split()) - prefix = check_output("git rev-parse --show-prefix".split()) - toplevel = check_output("git rev-parse --show-toplevel".split()) - head = check_output("git rev-parse HEAD".split()) - def merge_base_to_upstream(commit: str)->str: + def merge_base_to_upstream(commit: str) -> str: return subprocess.check_output(f"git merge-base {head} $(git for-each-ref refs/remotes/origin --format='%(objectname)')", cwd=srcdir,shell=True).decode().strip() if args.use_package_git_hash: @@ -417,7 +414,7 @@ def ros2nix(args): merge_base = merge_base_to_upstream(head) head = check_output(f"git rev-list {merge_base} -1 -- .".split()) - if not args.use_package_git_hash and toplevel in git_cache: #only use cache if not using seperate checkout per package + if not args.use_package_git_hash and toplevel in git_cache: #only use cache if not using separate checkout per package info = git_cache[toplevel] upstream_rev = info["rev"] else: @@ -428,7 +425,13 @@ def ros2nix(args): upstream_rev = merge_base_to_upstream(head) info = json.loads( subprocess.check_output( - ["nix-prefetch-git", "--quiet"]+ (["--sparse-checkout", prefix] if (prefix and args.use_package_git_hash) else [])+[ toplevel, upstream_rev], + ["nix-prefetch-git", "--quiet"] + + ( + ["--sparse-checkout", prefix] + if prefix and args.use_package_git_hash + else [] + ) + + [toplevel, upstream_rev], ).decode() ) git_cache[toplevel] = info @@ -442,7 +445,7 @@ def ros2nix(args): owner = "{match["owner"]}"; repo = "{match["repo"]}"; rev = "{info["rev"]}"; - sha256 = "{info["sha256"]}"; + sha256 = "{info["sha256"]}"; {sparse_checkout} }}''').strip() else: From f2893e44b9e16848533e03520b952cec9523d7a5 Mon Sep 17 00:00:00 2001 From: Michal Sojka Date: Fri, 7 Feb 2025 17:14:58 +0100 Subject: [PATCH 4/7] Don't generate extra empty lines in src expressions --- ros2nix/ros2nix.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/ros2nix/ros2nix.py b/ros2nix/ros2nix.py index 6b7345e..180b85b 100755 --- a/ros2nix/ros2nix.py +++ b/ros2nix/ros2nix.py @@ -213,6 +213,9 @@ def comma_separated(arg: str) -> list[str]: return [i.strip() for i in arg.split(",")] +def strip_empty_lines(text: str) -> str: + return os.linesep.join([s for s in text.splitlines() if s and not s.isspace()]) + def ros2nix(args): parser = argparse.ArgumentParser( prog="ros2nix", formatter_class=argparse.ArgumentDefaultsHelpFormatter @@ -440,23 +443,23 @@ def ros2nix(args): sparse_checkout = f"sparseCheckout = [\"{prefix}\"];" if (prefix and args.use_package_git_hash) else "" if match is not None: kwargs["src_param"] = "fetchFromGitHub" - kwargs["src_expr"] = dedent(f''' + kwargs["src_expr"] = strip_empty_lines(dedent(f''' fetchFromGitHub {{ owner = "{match["owner"]}"; repo = "{match["repo"]}"; rev = "{info["rev"]}"; sha256 = "{info["sha256"]}"; {sparse_checkout} - }}''').strip() + }}''')).strip() else: kwargs["src_param"] = "fetchgit" - kwargs["src_expr"] = dedent(f''' + kwargs["src_expr"] = strip_empty_lines(dedent(f''' fetchgit {{ url = "{url}"; rev = "{info["rev"]}"; sha256 = "{info["sha256"]}"; {sparse_checkout} - }}''').strip() + }}''')).strip() if prefix: # kwargs["src_expr"] = f'''let fullSrc = {kwargs["src_expr"]}; in "${{fullSrc}}/{prefix}"''' From 3567e9e85ae3da897ebe4c85badaf8fdea06c185 Mon Sep 17 00:00:00 2001 From: Max Beutelspacher Date: Sat, 8 Feb 2025 14:15:57 +0100 Subject: [PATCH 5/7] Improve argument name for per-package-src Co-authored-by: Michal Sojka --- README.md | 15 +++++++-------- ros2nix/ros2nix.py | 14 +++++++------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 2df8b7f..5ec9406 100644 --- a/README.md +++ b/README.md @@ -113,7 +113,7 @@ the [Autoware][] project as an example. ``` usage: ros2nix [-h] [--output OUTPUT | --output-as-ros-pkg-name | --output-as-nix-pkg-name] - [--output-dir OUTPUT_DIR] [--fetch] [--use-package-git-hash] + [--output-dir OUTPUT_DIR] [--fetch] [--use-per-package-src] [--patches | --no-patches] [--distro DISTRO] [--src-param SRC_PARAM] [--source-root SOURCE_ROOT] [--do-check] [--extra-build-inputs DEP1,DEP2,...] @@ -149,13 +149,12 @@ options: determined from the local git work tree. sourceRoot attribute is set if needed and not overridden by --source-root. (default: False) - --use-package-git-hash - When using --fetch, use the git hash of the package - sub-directory instead of the one of the upstream - repo.This will lead to longer generation time and - multiple source checkouts when building but will safe - rebuilds of packages that have not changed. (default: - False) + --use-per-package-src + When using --fetch, fetch only the package sub- + directory instead of the whole repo. For repos with + multiple packages, this will avoid rebuilds of + unchanged packages at the cost of longer generation + time. (default: False) --patches, --no-patches Add local git commits not present in git remote named "origin" to patches in the generated Nix expression. diff --git a/ros2nix/ros2nix.py b/ros2nix/ros2nix.py index 180b85b..6e080ad 100755 --- a/ros2nix/ros2nix.py +++ b/ros2nix/ros2nix.py @@ -252,10 +252,10 @@ def ros2nix(args): ) parser.add_argument( - "--use-package-git-hash", + "--use-per-package-src", action="store_true", - help="When using --fetch, use the git hash of the package sub-directory instead of the one of the upstream repo." - "This will lead to longer generation time and multiple source checkouts when building but will safe rebuilds of packages that have not changed." + help="When using --fetch, fetch only the package sub-directory instead of the whole repo. " + "For repos with multiple packages, this will avoid rebuilds of unchanged packages at the cost of longer generation time." ) parser.add_argument( "--patches", @@ -412,12 +412,12 @@ def ros2nix(args): def merge_base_to_upstream(commit: str) -> str: return subprocess.check_output(f"git merge-base {head} $(git for-each-ref refs/remotes/origin --format='%(objectname)')", cwd=srcdir,shell=True).decode().strip() - if args.use_package_git_hash: + if args.use_per_package_src: # we need to get merge_base again to filter out applied patches from the package git hash merge_base = merge_base_to_upstream(head) head = check_output(f"git rev-list {merge_base} -1 -- .".split()) - if not args.use_package_git_hash and toplevel in git_cache: #only use cache if not using separate checkout per package + if not args.use_per_package_src and toplevel in git_cache: # only use cache if not using separate checkout per package info = git_cache[toplevel] upstream_rev = info["rev"] else: @@ -431,7 +431,7 @@ def ros2nix(args): ["nix-prefetch-git", "--quiet"] + ( ["--sparse-checkout", prefix] - if prefix and args.use_package_git_hash + if prefix and args.use_per_package_src else [] ) + [toplevel, upstream_rev], @@ -440,7 +440,7 @@ def ros2nix(args): git_cache[toplevel] = info match = re.match("https://github.com/(?P[^/]*)/(?P.*?)(.git|/.*)?$", url) - sparse_checkout = f"sparseCheckout = [\"{prefix}\"];" if (prefix and args.use_package_git_hash) else "" + sparse_checkout = f"sparseCheckout = [\"{prefix}\"];" if (prefix and args.use_per_package_src) else "" if match is not None: kwargs["src_param"] = "fetchFromGitHub" kwargs["src_expr"] = strip_empty_lines(dedent(f''' From 50537092286ddc3e2e2751ac9d22a1002a9334af Mon Sep 17 00:00:00 2001 From: Michal Sojka Date: Sun, 9 Feb 2025 10:46:55 +0100 Subject: [PATCH 6/7] Use "non-cone mode" for --use-per-package-src In cone mode (before this commit), the fetched sources contained not only the specified subdirectory, but also the files in all its parent directories. Non-cone mode means, that only the specified subdirectory is fetched. --- ros2nix/ros2nix.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ros2nix/ros2nix.py b/ros2nix/ros2nix.py index 6e080ad..5224254 100755 --- a/ros2nix/ros2nix.py +++ b/ros2nix/ros2nix.py @@ -430,7 +430,7 @@ def ros2nix(args): subprocess.check_output( ["nix-prefetch-git", "--quiet"] + ( - ["--sparse-checkout", prefix] + ["--sparse-checkout", prefix, "--non-cone-mode"] if prefix and args.use_per_package_src else [] ) @@ -440,7 +440,9 @@ def ros2nix(args): git_cache[toplevel] = info match = re.match("https://github.com/(?P[^/]*)/(?P.*?)(.git|/.*)?$", url) - sparse_checkout = f"sparseCheckout = [\"{prefix}\"];" if (prefix and args.use_per_package_src) else "" + sparse_checkout = f"""sparseCheckout = ["{prefix}"]; + nonConeMode = true;""" if prefix and args.use_per_package_src else "" + if match is not None: kwargs["src_param"] = "fetchFromGitHub" kwargs["src_expr"] = strip_empty_lines(dedent(f''' From 83c13b82096c3336a87a29d86c5b9f8fa91a98eb Mon Sep 17 00:00:00 2001 From: Michal Sojka Date: Sun, 9 Feb 2025 10:50:09 +0100 Subject: [PATCH 7/7] Add test for --use-per-package-src --- test/test.bats | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/test.bats b/test/test.bats index f9ee8c2..5dd829a 100644 --- a/test/test.bats +++ b/test/test.bats @@ -113,3 +113,12 @@ load common.bash assert_file_not_contains ./ros-node.nix library-patch\.patch nix-build -A rosPackages.jazzy.ros-node } + +@test "--use-per-package-src" { + git clone https://github.com/wentasah/ros2nix + ros2nix --output-as-nix-pkg-name --fetch --use-per-package-src $(find "ros2nix/test/ws/src" -name package.xml) + nix-build -A rosPackages.jazzy.ros-node + run ./result/lib/ros_node/node + assert_success + assert_line --partial "hello world" +}