Fix "make tar" and "make dist" #4179
Conversation
We changed directory to the wrong directory. This change also separates the preparation phase from the tarball building phase.
|
@meena-vyas, I would very much appreciate it if you had a look at this on Solaris. |
| @@ -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 - | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Nah. Set the whole TAR_COMMAND then. See .travis-create-release.sh, which does exactly that for Mac OS X.
There was a problem hiding this comment.
But you have "TARFLAGS". Oh well.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I am not saying stop. I am saying putting it in a variable lets someone easily override it if they have to.
There was a problem hiding this comment.
I think you and I don't quite understand "enforce" the same way ;-)
There was a problem hiding this comment.
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)...
There was a problem hiding this comment.
TAR_COMMAND, which should really be called TAR_CREATE
If not TAR_CREATE_STDOUT...
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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...
What's emoji for dropped jaw? I've explicitly checked Solaris manual page yesterday, before suggesting to replace egrep... Though it was quite old version... Yeah, newer page says -E is not an option. Wow! To recap, the background for suggestion to replace egrep with grep -E was that egrep is listed as deprecated [at least on Linux, Tru64 and HP-UX], but apparently Solaris goes |
for /usr/bin/grep. But it is for /usr/xpg4/bin/grep. [And actually even old page says that. I suppose I was not attentive enough yesterday. Double-apologies, Richard.] BTW, Solaris manual page for egrep says that /usr/bin/egrep recognizes | between full expressions. Double-checking what does it mean in practice... |
|
Just switch to egrep seems to work. I mean adjustment to the expression doesn't seem to be required. |
Well, one can actually do exactly that as it is now (modulo egrep thing). One probably should recognize that gtar is in an add-on package, i.e. one probably can't assume that it's available on all installations. |
|
Cool. I'll do the change back to egrep a little later |
|
|
dot-asm
added
the
approval: done
Reviewed-by: Andy Polyakov <appro@openssl.org> (Merged from #4179)
We changed directory to the wrong directory. This change also separates the preparation phase from the tarball building phase. Reviewed-by: Andy Polyakov <appro@openssl.org> (Merged from #4179)
Reviewed-by: Andy Polyakov <appro@openssl.org> (Merged from #4179)
The "tar" make target wasn't quite right, it didn't change directory correctly, making the preparation command given in the "dist" target useless. This also clarifies that a tar command that understands
--ownerand--groupis expected.This is an alternative to the fix in #4114