Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'make dist' creates empty tarball on macOS #216

Open
apjanke opened this issue Mar 25, 2019 · 14 comments · Fixed by #219 or #218
Open

'make dist' creates empty tarball on macOS #216

apjanke opened this issue Mar 25, 2019 · 14 comments · Fixed by #219 or #218

Comments

@apjanke
Copy link
Contributor

apjanke commented Mar 25, 2019

From current master, on macOS 10.14.3:

$ make dist        
mkdir -p "tmp"
Creating package version 0.7.0 release ...
rm -f -r "tmp/doctest-0.7.0"
git archive --format=tar --prefix="tmp/doctest-0.7.0/" HEAD | tar -x
rm -f "tmp/doctest-0.7.0/README.matlab.md" \
	      "tmp/doctest-0.7.0/.gitignore" \
	      "tmp/doctest-0.7.0/.travis.yml" \
	      "tmp/doctest-0.7.0/.mailmap"
rm -f -r "tmp/doctest-0.7.0/util"
chmod -R a+rX,u+w,go-w "tmp/doctest-0.7.0"
tar: Option --mtime=2019-03-23 12:31:42 -0300 is not supported
Usage:
  List:    tar -tf <archive-filename>
  Extract: tar -xf <archive-filename>
  Create:  tar -cf <archive-filename> [filenames...]
  Help:    tar --help
$ echo $?      
0

Looks like it's expecting GNU tar and not compatible with BSD tar? And this failure isn't propagating up to a failure of the make dist call itself?

$ sw_vers      
ProductName:	Mac OS X
ProductVersion:	10.14.3
BuildVersion:	18D109
$ which tar  
/usr/bin/tar
$ tar --version   
bsdtar 2.8.3 - libarchive 2.8.3
@catch22
Copy link
Collaborator

catch22 commented Mar 28, 2019

This bug was hidden by another bug (#189). @oheim rewrote the Makefile some years ago, likely only having GNU tar in mind. Here is a workaround until this gets resolved:

brew install gnu-tar
PATH="/usr/local/opt/gnu-tar/libexec/gnubin:$PATH" make dist

Not sure about the error propagation issue.

@cbm755
Copy link
Collaborator

cbm755 commented Mar 28, 2019

IIRC, he was not concerned with portability because the Maintainer's Makefile was only to be used by developers.

That isn't to say we can't change it, just that there may be other places in the Makefile that use GNU extensions.

In some sense, the error propagation seems more serious.

@catch22
Copy link
Collaborator

catch22 commented Mar 28, 2019

I'm using macOS for development (but have been using the gnubin workaround).

@cbm755
Copy link
Collaborator

cbm755 commented Mar 28, 2019

I should have said "... and developers on macOS would be able to figure out workarounds."

Assigning to you b/c I don't know anything about macOS.

@mtmiller
Copy link
Collaborator

Yeah, agree that the bigger issue seems to be the error being suppressed. If macOS users can do make dist TAR=gtar for it to work, then the main issue would be ensuring that make sees the error if it fails.

@apjanke
Copy link
Contributor Author

apjanke commented Mar 29, 2019

If macOS users can do make dist TAR=gtar for it to work

This makes it a little harder to automate testing. What if we wanted to write a script that cloned and did make dist for all Octave Forge repos, to make sure they're building? Do we assume that GNU tar is required for all Forge packages, and pass TAR=gtar for everything when building on macOS?

Here's a patch that will both enable the TAR=gtar workaround and automatically pick up GNU tar on BSD systems where it is installed as gtar. #218

In some sense, the error propagation seems more serious.

I agree.

I believe the error suppression is due to the use of the pipeline, which only returns the exit status of the last command in the pipeline.

[octave-doctest] $ echo foo | false | true
[octave-doctest] $ echo $?
0

I think you'll need to check $PIPESTATUS to get the error to propagate. Or replace the pipeline with a multi-step operation that uses temporary files.

@cbm755
Copy link
Collaborator

cbm755 commented Mar 29, 2019

Would it be enough to pull the gzip bit off of the pipeline? Its the last element so... so would tar then fail?

Then we could have a separate gzip target that gzips's the result.

Just thinking out loud, most of this is beyond my meager Makefile abilities.

@apjanke
Copy link
Contributor Author

apjanke commented Mar 29, 2019

Would it be enough to pull the gzip bit off of the pipeline? Its the last element so... so would tar then fail?

Yes.

@mtmiller
Copy link
Collaborator

Yes, the following command creates an empty archive and the pipeline exits with success

tar -c --no-such-option file.txt | gzip -9n > foo.tar.gz

while the following does exit with failure, but creates an empty tar file

tar -c --no-such-option file.txt > foo.tar

And lastly, the following exits with failure and does not create an intermediate file that would need to be deleted later

tar -c --no-such-option -f foo.tar file.txt

Despite all that, I would probably prefer the original pipeline even though it may unexpectedly fail, just to avoid the mess of creating intermediate files, compressing them, deleting leftovers, etc.

$PIPESTATUS is not safe to use because it's a bash-specific feature.

Another option might be to keep the pipeline as is, but run

tar -tzf "$(2)" | grep some-known-file-name > /dev/null

to validate that the tarball exists and contains at least one known file name.

@catch22
Copy link
Collaborator

catch22 commented Mar 29, 2019

@mtmiller: Since we are anyways forcing the shell to be bash, can we not use $PIPESTATUS?

https://github.com/catch22/octave-doctest/blob/dddb940df7dee321d3c8c17ff26ee2c425450f12/Makefile#L1

@mtmiller
Copy link
Collaborator

@mtmiller: Since we are anyways forcing the shell to be bash, can we not use $PIPESTATUS?

Yeah, good point 😏

@catch22
Copy link
Collaborator

catch22 commented Apr 8, 2019

Reopening because the error suppression problem is not yet fixed. @apjanke, are you planning to take a stab at it? It seems like $PIPESTATUS is a the (a) way to go.

@apjanke
Copy link
Contributor Author

apjanke commented Apr 11, 2019

Better idea: the pipefail option.

The exit status of a pipeline is the exit status of the last command in the pipeline, unless the pipefail option is enabled (see The Set Builtin). If pipefail is enabled, the pipeline’s return status is the value of the last (rightmost) command to exit with a non-zero status, or zero if all commands exit successfully.

Here's a PR: #219

@cbm755
Copy link
Collaborator

cbm755 commented Jul 11, 2022

This look good. However, it reveals another problem, namely that even when the tar call fails (imagine macOS without gtar installed) it will create an empty tar file, so another invocation of make thinks that this target has been built successfully:

This remains to be addressed from #219.

@cbm755 cbm755 reopened this Jul 11, 2022
@cbm755 cbm755 modified the milestones: 0.8.0, 0.9.0 Jul 11, 2022
@cbm755 cbm755 removed this from the 0.9.0 milestone Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants