diff mbox series

scripts/archive-source: find directory name for subprojects

Message ID 20241010150652.1814677-1-pbonzini@redhat.com (mailing list archive)
State New, archived
Headers show
Series scripts/archive-source: find directory name for subprojects | expand

Commit Message

Paolo Bonzini Oct. 10, 2024, 3:06 p.m. UTC
Rust subprojects have the semantic version (followed by -rs) in the subproject
name, but the full version (without -rs) is used by crates.io for the root
directory of the tarball.  Teach scripts/archive-source.sh to look for the
root directory name in wrap files.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/archive-source.sh | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

Comments

Peter Maydell Oct. 10, 2024, 4:01 p.m. UTC | #1
On Thu, 10 Oct 2024 at 16:07, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> Rust subprojects have the semantic version (followed by -rs) in the subproject
> name, but the full version (without -rs) is used by crates.io for the root
> directory of the tarball.  Teach scripts/archive-source.sh to look for the
> root directory name in wrap files.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  scripts/archive-source.sh | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
> index 65af8063e4b..7c7727eab58 100755
> --- a/scripts/archive-source.sh
> +++ b/scripts/archive-source.sh
> @@ -48,13 +48,33 @@ function tree_ish() {
>      echo "$retval"
>  }
>
> +function subproject_dir() {

"function" here is unnecessary and a bashism, but I guess that's
the way the rest of the script is written.

> +    if test -f subprojects/$1.wrap; then

We should quote $1

> +      # Print the directory key of the wrap file, defaulting to the subproject name

Probably worth commenting here what the format of the wrap
file is (i.e. that it is a toml file and we are looking for
the "directory" key in a [wrap-something] section).

> +      local dir=$(sed \

https://www.shellcheck.net/wiki/SC2155
says we should write this
         local dir
         dir = "$(sed .... \
                ... \
                subprojects/"$1".wrap)"

to avoid losing the exit status of sed.

> +        -ne '/^\[wrap-[a-z][a-z]*\]$/!b' \
> +        -e ':label' \
> +        -e 'n' \
> +        -e 's/^directory *= *//p' \
> +        -e 'tquit' \
> +        -e '/^\[$/!blabel' \
> +        -e ':quit' \
> +        -e 'q' \
> +        subprojects/$1.wrap)

You could also write this

      local dir="$(sed -ne \
        '/^\[wrap-[a-z][a-z]*\]$/,/^\[/s/^directory *= *//p' \
        subprojects/"$1".wrap)"

i.e. use a sed address-range to say "operate only on
lines between [wrap-foo] and the next line starting '[',
and within that range print out the value of the directory
key". Definitely shorter. Is it more readable? IDK.
Behaviour in corner cases (multiple [wrap-foo] sections,
multiple 'directory' key lines) is different, but none of
our wrap files have those weirdnesses and I don't think
they're valid wrapfile syntax anyway.

> +      echo "${dir-$1}"

I think you want "${dir:-$1}" here, so that we get the
argument both if 'dir' is unset and if it is the
empty string. Otherwise (at least in my local testing)
you get the wrong thing for passing it 'keycodemapdb' or
other wrapfiles with no directory= key.

> +    else
> +      echo "error: scripts/archive-source.sh should only process wrap subprojects" 2>&1
> +      exit 1
> +    fi

Personally I think
     if test ! -f subprojects/"$1".wrap; then
        echo "error: ..."
        exit
     fi
     # normal handling here

is a bit more readable than deferring the error-exit to
the end of the function.

> +}
> +
>  git archive --format tar "$(tree_ish)" > "$tar_file"
>  test $? -ne 0 && error "failed to archive qemu"
>
>  for sp in $subprojects; do
>      meson subprojects download $sp
>      test $? -ne 0 && error "failed to download subproject $sp"
> -    tar --append --file "$tar_file" --exclude=.git subprojects/$sp
> +    tar --append --file "$tar_file" --exclude=.git subprojects/$(subproject_dir $sp)

We should quote: subprojects/"$(subproject_dir "$sp")"

>      test $? -ne 0 && error "failed to append subproject $sp to $tar_file"
>  done
>  exit 0
> --
> 2.46.2

thanks
-- PMM
diff mbox series

Patch

diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
index 65af8063e4b..7c7727eab58 100755
--- a/scripts/archive-source.sh
+++ b/scripts/archive-source.sh
@@ -48,13 +48,33 @@  function tree_ish() {
     echo "$retval"
 }
 
+function subproject_dir() {
+    if test -f subprojects/$1.wrap; then
+      # Print the directory key of the wrap file, defaulting to the subproject name
+      local dir=$(sed \
+        -ne '/^\[wrap-[a-z][a-z]*\]$/!b' \
+        -e ':label' \
+        -e 'n' \
+        -e 's/^directory *= *//p' \
+        -e 'tquit' \
+        -e '/^\[$/!blabel' \
+        -e ':quit' \
+        -e 'q' \
+        subprojects/$1.wrap)
+      echo "${dir-$1}"
+    else
+      echo "error: scripts/archive-source.sh should only process wrap subprojects" 2>&1
+      exit 1
+    fi
+}
+
 git archive --format tar "$(tree_ish)" > "$tar_file"
 test $? -ne 0 && error "failed to archive qemu"
 
 for sp in $subprojects; do
     meson subprojects download $sp
     test $? -ne 0 && error "failed to download subproject $sp"
-    tar --append --file "$tar_file" --exclude=.git subprojects/$sp
+    tar --append --file "$tar_file" --exclude=.git subprojects/$(subproject_dir $sp)
     test $? -ne 0 && error "failed to append subproject $sp to $tar_file"
 done
 exit 0