On Thu, 10 Oct 2024 at 16:07, Paolo Bonzini <pbonz...@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 <pbonz...@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