diff mbox series

scripts/release: Release also tarball with everything

Message ID 20210129200034.205263-1-plautrba@redhat.com (mailing list archive)
State Rejected
Headers show
Series scripts/release: Release also tarball with everything | expand

Commit Message

Petr Lautrbach Jan. 29, 2021, 8 p.m. UTC
Create and publish with sha256sum also tarball called
selinux-$VERS.tar.gz with the whole tree. It could be useful for unit
testing directly from tarball or backporting patches which affects more
subdirectories. Github already provides similar archive called "Source
code (tar.gz)" via release assets, but there's no guarantee this file
would not change.

Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
---
 scripts/release | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

Comments

Nicolas Iooss Jan. 30, 2021, 1:32 p.m. UTC | #1
On Fri, Jan 29, 2021 at 9:06 PM Petr Lautrbach <plautrba@redhat.com> wrote:
>
> Create and publish with sha256sum also tarball called
> selinux-$VERS.tar.gz with the whole tree. It could be useful for unit
> testing directly from tarball or backporting patches which affects more
> subdirectories. Github already provides similar archive called "Source
> code (tar.gz)" via release assets, but there's no guarantee this file
> would not change.
>
> Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
> ---
>  scripts/release | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/release b/scripts/release
> index 895a0e1ca1a1..40a9c06f56b9 100755
> --- a/scripts/release
> +++ b/scripts/release
> @@ -35,6 +35,8 @@ for i in $DIRS_NEED_PREFIX; do
>         cd ..
>  done
>
> +git archive -o $DEST/selinux-$VERS.tar.gz --prefix=selinux-$VERS/ $VERS
> +
>  cd $DEST
>
>  git add .
> @@ -54,13 +56,28 @@ echo ""
>  echo "[short log](https://github.com/SELinuxProject/selinux/releases/download/$RELEASE_TAG/shortlog-$RELEASE_TAG.txt)"
>  echo ""
>
> -for i in *.tar.gz; do
> +for i in $DIRS; do
> +       tarball=$i-$VERS.tar.gz
> +       echo -n "[$tarball](https://github.com/SELinuxProject/selinux/releases/download/$RELEASE_TAG/$tarball) "
> +       sha256sum $tarball | cut -d " " -f 1
> +       echo ""
> +done
>
> -       echo -n "[$i](https://github.com/SELinuxProject/selinux/releases/download/$RELEASE_TAG/$i) "
> -       sha256sum $i | cut -d " " -f 1
> +for i in $DIRS_NEED_PREFIX; do
> +       tarball=selinux-$i-$VERS.tar.gz
> +       echo -n "[$tarball](https://github.com/SELinuxProject/selinux/releases/download/$RELEASE_TAG/$tarball) "
> +       sha256sum $tarball | cut -d " " -f 1
>         echo ""
>  done
>
> +echo "### Everything"
> +
> +echo ""
> +
> +echo -n "[selinux-$VERS.tar.gz](https://github.com/SELinuxProject/selinux/releases/download/$RELEASE_TAG/selinux-$VERS.tar.gz) "
> +sha256sum selinux-$VERS.tar.gz | cut -d " " -f 1
> +echo ""

Hello, there are at least two issues here:
* The section is named "Everything" but on
https://github.com/SELinuxProject/selinux/wiki/Releases it is named
"All in one". What is the proper name?
* $VERS comes from a VERSION file in a subdirectory. It would be more
consistent to either use $RELEASE_TAG (which is the content of the
main VERSION file) or to use a single $VERS variable (and to verify
that the */VERSION files all contain the same content, for example
with "diff VERSION $i/VERSION"). Which option would be preferable?

Moreover I do not like using string variables without quotes (and
shellcheck reports warnings for this), but this is a general issue of
this script. I will send a patch to improve the release script.

Cheers,
Nicolas
Nicolas Iooss Jan. 31, 2021, 5:30 p.m. UTC | #2
On Sun, Jan 31, 2021 at 11:09 AM Petr Lautrbach <plautrba@redhat.com> wrote:
>
> Nicolas Iooss <nicolas.iooss@m4x.org> writes:
>
> > On Fri, Jan 29, 2021 at 9:06 PM Petr Lautrbach <plautrba@redhat.com> wrote:
> >>
> >> Create and publish with sha256sum also tarball called
> >> selinux-$VERS.tar.gz with the whole tree. It could be useful for unit
> >> testing directly from tarball or backporting patches which affects more
> >> subdirectories. Github already provides similar archive called "Source
> >> code (tar.gz)" via release assets, but there's no guarantee this file
> >> would not change.
> >>
> >> Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
> >> ---
> >>  scripts/release | 23 ++++++++++++++++++++---
> >>  1 file changed, 20 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/scripts/release b/scripts/release
> >> index 895a0e1ca1a1..40a9c06f56b9 100755
> >> --- a/scripts/release
> >> +++ b/scripts/release
> >> @@ -35,6 +35,8 @@ for i in $DIRS_NEED_PREFIX; do
> >>         cd ..
> >>  done
> >>
> >> +git archive -o $DEST/selinux-$VERS.tar.gz --prefix=selinux-$VERS/ $VERS
> >> +
> >>  cd $DEST
> >>
> >>  git add .
> >> @@ -54,13 +56,28 @@ echo ""
> >>  echo "[short log](https://github.com/SELinuxProject/selinux/releases/download/$RELEASE_TAG/shortlog-$RELEASE_TAG.txt)"
> >>  echo ""
> >>
> >> -for i in *.tar.gz; do
> >> +for i in $DIRS; do
> >> +       tarball=$i-$VERS.tar.gz
> >> +       echo -n "[$tarball](https://github.com/SELinuxProject/selinux/releases/download/$RELEASE_TAG/$tarball) "
> >> +       sha256sum $tarball | cut -d " " -f 1
> >> +       echo ""
> >> +done
> >>
> >> -       echo -n "[$i](https://github.com/SELinuxProject/selinux/releases/download/$RELEASE_TAG/$i) "
> >> -       sha256sum $i | cut -d " " -f 1
> >> +for i in $DIRS_NEED_PREFIX; do
> >> +       tarball=selinux-$i-$VERS.tar.gz
> >> +       echo -n "[$tarball](https://github.com/SELinuxProject/selinux/releases/download/$RELEASE_TAG/$tarball) "
> >> +       sha256sum $tarball | cut -d " " -f 1
> >>         echo ""
> >>  done
> >>
> >> +echo "### Everything"
> >> +
> >> +echo ""
> >> +
> >> +echo -n "[selinux-$VERS.tar.gz](https://github.com/SELinuxProject/selinux/releases/download/$RELEASE_TAG/selinux-$VERS.tar.gz) "
> >> +sha256sum selinux-$VERS.tar.gz | cut -d " " -f 1
> >> +echo ""
> >
> > Hello, there are at least two issues here:
> > * The section is named "Everything" but on
> > https://github.com/SELinuxProject/selinux/wiki/Releases it is named
> > "All in one". What is the proper name?
>
>
> I used "All in one" in rc1 release but I didn't announce it properly and
> it didn't feel good to me. So I sent this patch to make this official.
> And I like "Everything" more than the original "All in one". I can
> change it on the release page if the change is accepted or drop it
> completely if it's denied.

I find "Everything" to be quite unclear, because for example the
archive does not contain the git history... What about "Source
repository snapshot", which better describes what the archive really
is? This is a personal opinion and I am also fine with keeping
"Everything" if you do not like my suggestion.

> > * $VERS comes from a VERSION file in a subdirectory. It would be more
> > consistent to either use $RELEASE_TAG (which is the content of the
> > main VERSION file) or to use a single $VERS variable (and to verify
> > that the */VERSION files all contain the same content, for example
> > with "diff VERSION $i/VERSION"). Which option would be preferable?
>
> For now I'd use $RELEASE_TAG for selinux-X.Y.tar.gz
>
> and maybe something like:
>
> -for i in $DIRS; do
> -       tarball=$i-$VERS.tar.gz
> +for i in $DIRS; do
> +       tarball=`ls $i-*.tar.gz`

I do not like using wildcard patterns. In the patch I sent
(https://lore.kernel.org/selinux/20210130133313.1759011-1-nicolas.iooss@m4x.org/)
I replaced this part with reading $i/VERSION again:

+for COMPONENT in "${DIRS[@]}"; do
+ DIR="${COMPONENT#selinux-}"
+ VERS="$(cat "$DIR/VERSION")"
+ TAG="$COMPONENT-$VERS"
+ tarball="$TAG.tar.gz"

Nevertheless the script is currently using wildcards and your
suggestion works, so you can use `ls $i-*.tar.gz` and I will then
propose a new version of my patch to remove this. Or you can include
my changes in your patch, if you agree with them. How do you want to
proceed?

Thanks,
Nicolas
Petr Lautrbach Feb. 1, 2021, 1:58 p.m. UTC | #3
Nicolas Iooss <nicolas.iooss@m4x.org> writes:

> On Sun, Jan 31, 2021 at 11:09 AM Petr Lautrbach <plautrba@redhat.com> wrote:
>>
>> Nicolas Iooss <nicolas.iooss@m4x.org> writes:
>>
>> > On Fri, Jan 29, 2021 at 9:06 PM Petr Lautrbach <plautrba@redhat.com> wrote:
>> >>
>> >> Create and publish with sha256sum also tarball called
>> >> selinux-$VERS.tar.gz with the whole tree. It could be useful for unit
>> >> testing directly from tarball or backporting patches which affects more
>> >> subdirectories. Github already provides similar archive called "Source
>> >> code (tar.gz)" via release assets, but there's no guarantee this file
>> >> would not change.
>> >>
>> >> Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
>> >> ---
>> >>  scripts/release | 23 ++++++++++++++++++++---
>> >>  1 file changed, 20 insertions(+), 3 deletions(-)
>> >>
>> >> diff --git a/scripts/release b/scripts/release
>> >> index 895a0e1ca1a1..40a9c06f56b9 100755
>> >> --- a/scripts/release
>> >> +++ b/scripts/release
>> >> @@ -35,6 +35,8 @@ for i in $DIRS_NEED_PREFIX; do
>> >>         cd ..
>> >>  done
>> >>
>> >> +git archive -o $DEST/selinux-$VERS.tar.gz --prefix=selinux-$VERS/ $VERS
>> >> +
>> >>  cd $DEST
>> >>
>> >>  git add .
>> >> @@ -54,13 +56,28 @@ echo ""
>> >>  echo "[short log](https://github.com/SELinuxProject/selinux/releases/download/$RELEASE_TAG/shortlog-$RELEASE_TAG.txt)"
>> >>  echo ""
>> >>
>> >> -for i in *.tar.gz; do
>> >> +for i in $DIRS; do
>> >> +       tarball=$i-$VERS.tar.gz
>> >> +       echo -n "[$tarball](https://github.com/SELinuxProject/selinux/releases/download/$RELEASE_TAG/$tarball) "
>> >> +       sha256sum $tarball | cut -d " " -f 1
>> >> +       echo ""
>> >> +done
>> >>
>> >> -       echo -n "[$i](https://github.com/SELinuxProject/selinux/releases/download/$RELEASE_TAG/$i) "
>> >> -       sha256sum $i | cut -d " " -f 1
>> >> +for i in $DIRS_NEED_PREFIX; do
>> >> +       tarball=selinux-$i-$VERS.tar.gz
>> >> +       echo -n "[$tarball](https://github.com/SELinuxProject/selinux/releases/download/$RELEASE_TAG/$tarball) "
>> >> +       sha256sum $tarball | cut -d " " -f 1
>> >>         echo ""
>> >>  done
>> >>
>> >> +echo "### Everything"
>> >> +
>> >> +echo ""
>> >> +
>> >> +echo -n "[selinux-$VERS.tar.gz](https://github.com/SELinuxProject/selinux/releases/download/$RELEASE_TAG/selinux-$VERS.tar.gz) "
>> >> +sha256sum selinux-$VERS.tar.gz | cut -d " " -f 1
>> >> +echo ""
>> >
>> > Hello, there are at least two issues here:
>> > * The section is named "Everything" but on
>> > https://github.com/SELinuxProject/selinux/wiki/Releases it is named
>> > "All in one". What is the proper name?
>>
>>
>> I used "All in one" in rc1 release but I didn't announce it properly and
>> it didn't feel good to me. So I sent this patch to make this official.
>> And I like "Everything" more than the original "All in one". I can
>> change it on the release page if the change is accepted or drop it
>> completely if it's denied.
>
> I find "Everything" to be quite unclear, because for example the
> archive does not contain the git history... What about "Source
> repository snapshot", which better describes what the archive really
> is? This is a personal opinion and I am also fine with keeping
> "Everything" if you do not like my suggestion.

"Source repository snapshot" works for me.

>> > * $VERS comes from a VERSION file in a subdirectory. It would be more
>> > consistent to either use $RELEASE_TAG (which is the content of the
>> > main VERSION file) or to use a single $VERS variable (and to verify
>> > that the */VERSION files all contain the same content, for example
>> > with "diff VERSION $i/VERSION"). Which option would be preferable?
>>
>> For now I'd use $RELEASE_TAG for selinux-X.Y.tar.gz
>>
>> and maybe something like:
>>
>> -for i in $DIRS; do
>> -       tarball=$i-$VERS.tar.gz
>> +for i in $DIRS; do
>> +       tarball=`ls $i-*.tar.gz`
>
> I do not like using wildcard patterns. In the patch I sent
> (https://lore.kernel.org/selinux/20210130133313.1759011-1-nicolas.iooss@m4x.org/)
> I replaced this part with reading $i/VERSION again:
>
> +for COMPONENT in "${DIRS[@]}"; do
> + DIR="${COMPONENT#selinux-}"
> + VERS="$(cat "$DIR/VERSION")"
> + TAG="$COMPONENT-$VERS"
> + tarball="$TAG.tar.gz"
>
> Nevertheless the script is currently using wildcards and your
> suggestion works, so you can use `ls $i-*.tar.gz` and I will then
> propose a new version of my patch to remove this. Or you can include
> my changes in your patch, if you agree with them. How do you want to
> proceed?

I like your patch and I don't think it's neccessary to apply both. Let's
drop mine proposal. Please merge my changes into yours and update the
patch with "Source repository snapshot"

Thanks!





>
> Thanks,
> Nicolas
diff mbox series

Patch

diff --git a/scripts/release b/scripts/release
index 895a0e1ca1a1..40a9c06f56b9 100755
--- a/scripts/release
+++ b/scripts/release
@@ -35,6 +35,8 @@  for i in $DIRS_NEED_PREFIX; do
 	cd ..
 done
 
+git archive -o $DEST/selinux-$VERS.tar.gz --prefix=selinux-$VERS/ $VERS
+
 cd $DEST
 
 git add .
@@ -54,13 +56,28 @@  echo ""
 echo "[short log](https://github.com/SELinuxProject/selinux/releases/download/$RELEASE_TAG/shortlog-$RELEASE_TAG.txt)"
 echo ""
 
-for i in *.tar.gz; do
+for i in $DIRS; do
+	tarball=$i-$VERS.tar.gz
+	echo -n "[$tarball](https://github.com/SELinuxProject/selinux/releases/download/$RELEASE_TAG/$tarball) "
+	sha256sum $tarball | cut -d " " -f 1
+	echo ""
+done
 
-	echo -n "[$i](https://github.com/SELinuxProject/selinux/releases/download/$RELEASE_TAG/$i) "
-	sha256sum $i | cut -d " " -f 1
+for i in $DIRS_NEED_PREFIX; do
+	tarball=selinux-$i-$VERS.tar.gz
+	echo -n "[$tarball](https://github.com/SELinuxProject/selinux/releases/download/$RELEASE_TAG/$tarball) "
+	sha256sum $tarball | cut -d " " -f 1
 	echo ""
 done
 
+echo "### Everything"
+
+echo ""
+
+echo -n "[selinux-$VERS.tar.gz](https://github.com/SELinuxProject/selinux/releases/download/$RELEASE_TAG/selinux-$VERS.tar.gz) "
+sha256sum selinux-$VERS.tar.gz | cut -d " " -f 1
+echo ""
+
 echo "And then run:"
 echo "  cd $WIKIDIR"
 echo "  git commit  -m \"Release $RELEASE_TAG\" -a -s"