the-tcpdump-group / tcpslice

tcpslice concatenates multiple pcap files together, or extracts time slices from one or more pcap files.
63 stars 22 forks source link

configure: put autoconf-generated files into the release tarball. #17

Closed guyharris closed 1 year ago

infrastation commented 1 year ago

If the only information this process needs from the Makefile is RELEASE_FILES, this could be made much simpler by having a separate gentarball.sh script without the Makefile-incurred complications, roughly as follows:

#!/bin/sh -e
AUTOCONF_RELEASE_FILES=`configure config.h.in etc...'
test "$(git status --porcelain | wc -l)" -eq 0
${AUTORECONF:-autoreconf} -f
git archive $(make printreleasefilelist) $AUTOCONF_RELEASE_FILES

Then this script would not even put itself in the tarball in order not to create an impression that it is OK to create a new release tarball from an old release tarball. And when it is a separate file, it can be processed using shellcheck (both manually and automatically). Does it make sense?

infrastation commented 1 year ago

(Except git archive will not process AUTOCONF_RELEASE_FILES because these don't exist in the repository, but the idea should be clear.)

guyharris commented 1 year ago

I.e., you make a release tarball by doing ./gentarball.sh, and either 1) there would be no releasetar rule in Makefile.in or 2) that rule would just run gentarball.sh?

infrastation commented 1 year ago

(3). An error message to say that one needs to use a git clone and the script.

infrastation commented 1 year ago

Or (2) that passes RELEASE_FILES to the script as arguments, which does not include the script, and which has to fail with a useful message when this is an unpacked tarball.

fxlb commented 1 year ago

Can be now removed in releasecheck target:

echo "[$@] $$ ./autogen.sh" && \
./autogen.sh && \
fxlb commented 1 year ago

(Rebased.)

guyharris commented 1 year ago

Can be now removed in releasecheck target:

Done.

guyharris commented 1 year ago

Speaking of AIX, does anybody know of any public CI sites offering AIX?

guyharris commented 1 year ago

An error message to say that one needs to use a git clone and the script.

So the releasetar rule would be something such as

releasetar:
    if test -x ./gentarball.sh; then \
        ./gentarball.sh \
    else \
        echo {error message saying to use a cloned repository} 1>&2; \
        exit 1 \
    fi
fxlb commented 1 year ago

Currently, the releasetar target (in this PR) is only 20 lines, perfectly readable. Thus not sure we need to change to a gentarball.sh script.

fxlb commented 1 year ago

Speaking of AIX, does anybody know of any public CI sites offering AIX?

If AIX was out of scope, we could remove gzip and go back to tar czf...

fxlb commented 1 year ago

FYI: a way to debug is adding:

        @TAG=$(PROG)-`cat VERSION` && \
+       set -x && \
        AUTORECONF_DIR=/tmp/autoreconf_"$$TAG"_$$$$ && 
fxlb commented 1 year ago

Another way to see this tar / gzip use: There is no reason to build the archive of an AIX system, thus we can get rid of gzip and use tar czf.

infrastation commented 1 year ago

If the shell script lived outside of the Makefile and could be kept identical across all three projects (as is build_common.sh), that would make a bit more sense to me, but that's not the main point.

Modern versions of GNU tar will guess what you mean when you run tar caf archive.tar.gz or even tar cf archive.tar.gz, with (unlike Solaris tar) or without the dash before the command. Other tar implementations will not be as forgiving, and might not support the common convenience options for compression (e.g. AIX tar). That said, it would be fine not to support making a release tarball on less common OSes if they don't have the capability for that. That said, when the script cannot produce a well-formed tarball, it should not fail to fail -- that's the most important bit.

This is why to me it would make more sense to run tar first (and let sh -e guard it so I do not have to remember about &&) and then to run gzip separately, so I don't have to remember the POSIX way to spell set -o pipefail (and let sh -e guard it, so I don't have to remember about &&), and can specify the compression level explicitly.

But anyway, if we manage to make this migration without introducing regressions, the rest can be improved later as and when required.

infrastation commented 1 year ago

Let me suggest to get the essential logic right in tcpslice before propagating it into other repositories.

guyharris commented 1 year ago

This is why to me it would make more sense to run tar first ... and then to run gzip separately,

Done.

guyharris commented 1 year ago

and let sh -e guard it

Presumably meaning have the first command in the sequence be set -e.

The description of -e in the Single UNIX Standard is:

When this option is on, when any command fails (for any of the reasons listed in Consequences of Shell Errors or by returning an exit status greater than zero), the shell immediately shall exit, as if by executing the exit special built-in utility with no arguments, with the following exceptions:

  1. The failure of any individual command in a multi-command pipeline shall not cause the shell to exit. Only the failure of the pipeline itself shall be considered.

  2. The -e setting shall be ignored when executing the compound list following the while, until, if, or elif reserved word, a pipeline beginning with the ! reserved word, or any command of an AND-OR list other than the last.

  3. If the exit status of a compound command other than a subshell command was the result of a failure while -e was being ignored, then -e shall not apply to this command.

This requirement applies to the shell environment and each subshell environment separately. For example, in:

set -e; (false; echo one) | cat; echo two

the false command causes the subshell to exit without executing echo one; however, echo two is executed because the exit status of the pipeline (false; echo one) | cat is zero.

which seems to indicate that the if/then/else might be an issue.

guyharris commented 1 year ago

which seems to indicate that the if/then/else might be an issue.

No, it just means that the command whose success or failure is being tested by while, until, if, or elif fails, that won't cause the shell to quit, as it obviously shouldn't.

infrastation commented 1 year ago

Yes, exactly. My earlier comment means that at some point I started to use #!/bin/sh -e as the default shebang, so when an unguarded command fails, there's an automatic emergency stop.

infrastation commented 1 year ago

Thank you, let's try this revision and see how much improvement the scripts still require to be declared good enough.

infrastation commented 1 year ago

One follow-up this change requires is to update the installation notes to tell the user to run ./configure first (as before), and just before that to say that if the build is from a git checkout rather than from a tarball, another command will need to be run first.