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.

Sign up for GitHub

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

Jump to bottom

Fix "make tar" and "make dist" #4179

Closed
wants to merge 4 commits into from
Closed

Fix "make tar" and "make dist" #4179

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a3be67a
Turn on error sensitivity in the "tar" target
levitte Aug 17, 2017
90c83c1
Prepare tarball in dist directory
levitte Aug 17, 2017
7e6691d
Add a comment on expectations in the "tar" target
levitte Aug 17, 2017
b4dce49
[to be squashed] not all greps have -E, use egrep instead
levitte Aug 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 9 additions & 6 deletions Configurations/unix-Makefile.tmpl
View file
Open in desktop
Expand Up @@ -679,9 +679,12 @@ tags TAGS: FORCE

# Release targets (note: only available on Unix) #####################

# If your tar command doesn't support --owner and --group, make sure to
# use one that does, for example GNU tar
TAR_COMMAND=$(TAR) $(TARFLAGS) --owner 0 --group 0 -cvf -
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you put those flags into a make variable

TARFLAGS = --owner 0 -- group 0

then it's at least easier for folks to work around.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah. Set the whole TAR_COMMAND then. See .travis-create-release.sh, which does exactly that for Mac OS X.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But you have "TARFLAGS". Oh well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True. However, we have enforced user 0 and group 0 since the start of OpenSSL (a long time ago, we used tardy to do that)... I'm not sure now is the time to stop.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not saying stop. I am saying putting it in a variable lets someone easily override it if they have to.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you and I don't quite understand "enforce" the same way ;-)

Copy link
Contributor

@dot-asm dot-asm Aug 18, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose question here is if we want users to use this or not. If we don't, then all bets on obscurity and linux specifics are off. But if we do want users to use it, then we're better off making it more intuitive. And more intuitive would be to detect if tar is GNU and act accordingly, e.g. issue warning and omit non-standard flags, or terminate with "GNU tar only" message. And command replacement should be more convenient. E.g. on Solaris one should be able to do say make tar TAR=gtar, i.e. without having to figure out/remember that it's TAR_COMMAND, which should really be called TAR_CREATE (because that's what the operation it does)...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TAR_COMMAND, which should really be called TAR_CREATE

If not TAR_CREATE_STDOUT...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevertheless, if users are using the tar (or dist) target, it would be unwise to make sudden variable changes now. Chances are that there are scripts that depend on them (such as our own .travis-create-release.sh).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wasn't my intention to suggest TAR_COMMAND rename. [I suppose I should have written "which should really have been called"?] The implication was rather to detect if tar (or more specifically $(TAR)) is GNU and do something about that...

PREPARE_CMD=:
tar:
set -e; \
TMPDIR=/var/tmp/openssl-copy.$$$$; \
DISTDIR=$(NAME); \
mkdir -p $$TMPDIR/$$DISTDIR; \
Expand All @@ -690,17 +693,17 @@ tar:
excl_re="^(fuzz/corpora|`echo $$excl_re | sed -e 's/ /$$|/g'`\$$)"; \
echo "$$excl_re"; \
git ls-tree -r --name-only --full-tree HEAD \
| grep -v -E "$$excl_re" \
| egrep -v "$$excl_re" \
| while read F; do \
mkdir -p $$TMPDIR/$$DISTDIR/`dirname $$F`; \
cp $$F $$TMPDIR/$$DISTDIR/$$F; \
done); \
(cd $$TMPDIR; \
(cd $$TMPDIR/$$DISTDIR; \
$(PREPARE_CMD); \
find $$TMPDIR/$$DISTDIR -type d -print | xargs chmod 755; \
find $$TMPDIR/$$DISTDIR -type f -print | xargs chmod a+r; \
find $$TMPDIR/$$DISTDIR -type f -perm -0100 -print | xargs chmod a+x; \
$(TAR_COMMAND) $$DISTDIR) \
find . -type d -print | xargs chmod 755; \
find . -type f -print | xargs chmod a+r; \
find . -type f -perm -0100 -print | xargs chmod a+x); \
(cd $$TMPDIR; $(TAR_COMMAND) $$DISTDIR) \
| (cd $(SRCDIR); gzip --best > $(TARFILE).gz); \
rm -rf $$TMPDIR
cd $(SRCDIR); ls -l $(TARFILE).gz
Expand Down