★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/24551/filesNouvelle recherche | Portail wanayoo
Skip to content

/node

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

build: make .tar.xz creation opt-out, fail if no xz #24551

Closed
wants to merge 2 commits into from
Closed

build: make .tar.xz creation opt-out, fail if no xz #24551

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
28bf228
build: make tar.xz creation opt-out, fail if no xz
rvagg Nov 21, 2018
8665814
fixup! build: make tar.xz creation opt-out, fail if no xz
rvagg Nov 22, 2018
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.
+24 −8
Diff settings

Always

Just for now

View file
32 Makefile
@@ -810,13 +810,29 @@ BINARYNAME=$(TARNAME)-$(PLATFORM)-$(ARCH)
endif
BINARYTAR=$(BINARYNAME).tar
# OSX doesn't have xz installed by default, http://macpkg.sourceforge.net/
XZ=$(shell which xz > /dev/null 2>&1; echo $$?)
HAS_XZ ?= $(shell which xz > /dev/null 2>&1; [[ $$? = 0 ]] && echo 1 || echo 0)
# Supply SKIP_XZ=1 to explicitly skip .tar.xz creation
SKIP_XZ ?= 0
This conversation was marked as resolved by refack

This comment has been minimized.

Sign in to view
Copy link
@refack

refack Nov 21, 2018 •
edited

Member

Why not keep XZ as (pseudocode)

XZ = HAS_XZ != 0 && SKIP_XZ == 0

?

This comment has been minimized.

Sign in to view
Copy link
@rvagg

rvagg Nov 21, 2018

Author Member

what do I do with that then? we need to be able to differentiate between "have xz", "have xz but asked to skip", "don't have xz" and "don't have xz but have to skip". There needs to be a hard fail as well as a soft skip.

This comment has been minimized.

Sign in to view
Copy link
@refack

refack Nov 21, 2018

Member

Just to keep the churn low. It we compute the value of XZ we can keep the later guards as is.
Like

node/Makefile

Line 961 in ba2647a

ifeq ($(HAS_XZ) $(SKIP_XZ), 1 0)

etc.

This comment has been minimized.

Sign in to view
Copy link
@rvagg

rvagg Nov 22, 2018

Author Member

I'm not concerned about churn, I'd rather aim for grokkability in Makefile since it's already full of magic. So is your suggestion that, with the check-xz as it is in this PR (i.e. with HAS_XZ and SKIP_XZ intact), that we roll up the blocks surrounding the xz call under the XZ boolean?

ifeq ($(HAS_XZ) $(SKIP_XZ), 1 0) is a little esoteric, so perhaps that'd be a good change for grokkability sake.

This comment has been minimized.

Sign in to view
Copy link
@refack

refack Nov 22, 2018

Member

Yes. That "if" is very specific. If we restore XZ but now it's cumputed as ($(HAS_XZ) $(SKIP_XZ), 1 0) (even comment that it means "we have a working xz available") the lower code blocks will be simpler to grok.
XZ = $(shell [[ $(HAS_XZ) = 1 && $(SKIP_XZ) = 0 ]] && echo 1 || echo 0)
XZ_COMPRESSION ?= 9e
PKG=$(TARNAME).pkg
MACOSOUTDIR=out/macos

ifeq ($(SKIP_XZ), 1)
check-xz:

This comment has been minimized.

Sign in to view
Copy link
@refack

refack Nov 23, 2018

Member

Maybe it's because check-xz should be .PHONY?
@echo "SKIP_XZ=1 supplied, skipping .tar.xz creation"
else
ifeq ($(HAS_XZ), 1)
check-xz:
else
check-xz:
@echo "No xz command, cannot continue"
@exit 1
This conversation was marked as resolved by refack

This comment has been minimized.

Sign in to view
Copy link
@refack

refack Nov 21, 2018

Member

So this is the tl;dr?

This comment has been minimized.

Sign in to view
Copy link
@rvagg

rvagg Nov 21, 2018

Author Member

yep, although skip is important cause we need it on AIX
endif
endif

.PHONY: release-only
release-only:
release-only: check-xz
@if [ "$(DISTTYPE)" = "release" ] && `grep -q REPLACEME doc/api/*.md`; then \
echo 'Please update REPLACEME in Added: tags in doc/api/*.md (See doc/releases.md)' ; \
exit 1 ; \
@@ -942,7 +958,7 @@ $(TARBALL): release-only $(NODE_EXE) doc
tar -cf $(TARNAME).tar $(TARNAME)
$(RM) -r $(TARNAME)
gzip -c -f -9 $(TARNAME).tar > $(TARNAME).tar.gz
ifeq ($(XZ), 0)
ifeq ($(XZ), 1)
xz -c -f -$(XZ_COMPRESSION) $(TARNAME).tar > $(TARNAME).tar.xz
endif
$(RM) $(TARNAME).tar
@@ -956,7 +972,7 @@ tar-upload: tar
chmod 664 $(TARNAME).tar.gz
scp -p $(TARNAME).tar.gz $(STAGINGSERVER):nodejs/$(DISTTYPEDIR)/$(FULLVERSION)/$(TARNAME).tar.gz
ssh $(STAGINGSERVER) "touch nodejs/$(DISTTYPEDIR)/$(FULLVERSION)/$(TARNAME).tar.gz.done"
ifeq ($(XZ), 0)
ifeq ($(XZ), 1)
chmod 664 $(TARNAME).tar.xz
scp -p $(TARNAME).tar.xz $(STAGINGSERVER):nodejs/$(DISTTYPEDIR)/$(FULLVERSION)/$(TARNAME).tar.xz
ssh $(STAGINGSERVER) "touch nodejs/$(DISTTYPEDIR)/$(FULLVERSION)/$(TARNAME).tar.xz.done"
@@ -982,7 +998,7 @@ $(TARBALL)-headers: release-only
tar -cf $(TARNAME)-headers.tar $(TARNAME)
$(RM) -r $(TARNAME)
gzip -c -f -9 $(TARNAME)-headers.tar > $(TARNAME)-headers.tar.gz
ifeq ($(XZ), 0)
ifeq ($(XZ), 1)
xz -c -f -$(XZ_COMPRESSION) $(TARNAME)-headers.tar > $(TARNAME)-headers.tar.xz
endif
$(RM) $(TARNAME)-headers.tar
@@ -994,7 +1010,7 @@ tar-headers-upload: tar-headers
chmod 664 $(TARNAME)-headers.tar.gz
scp -p $(TARNAME)-headers.tar.gz $(STAGINGSERVER):nodejs/$(DISTTYPEDIR)/$(FULLVERSION)/$(TARNAME)-headers.tar.gz
ssh $(STAGINGSERVER) "touch nodejs/$(DISTTYPEDIR)/$(FULLVERSION)/$(TARNAME)-headers.tar.gz.done"
ifeq ($(XZ), 0)
ifeq ($(XZ), 1)
chmod 664 $(TARNAME)-headers.tar.xz
scp -p $(TARNAME)-headers.tar.xz $(STAGINGSERVER):nodejs/$(DISTTYPEDIR)/$(FULLVERSION)/$(TARNAME)-headers.tar.xz
ssh $(STAGINGSERVER) "touch nodejs/$(DISTTYPEDIR)/$(FULLVERSION)/$(TARNAME)-headers.tar.xz.done"
@@ -1019,7 +1035,7 @@ endif
tar -cf $(BINARYNAME).tar $(BINARYNAME)
$(RM) -r $(BINARYNAME)
gzip -c -f -9 $(BINARYNAME).tar > $(BINARYNAME).tar.gz
ifeq ($(XZ), 0)
ifeq ($(XZ), 1)
xz -c -f -$(XZ_COMPRESSION) $(BINARYNAME).tar > $(BINARYNAME).tar.xz
endif
$(RM) $(BINARYNAME).tar
@@ -1034,7 +1050,7 @@ binary-upload: binary
chmod 664 $(TARNAME)-$(OSTYPE)-$(ARCH).tar.gz
scp -p $(TARNAME)-$(OSTYPE)-$(ARCH).tar.gz $(STAGINGSERVER):nodejs/$(DISTTYPEDIR)/$(FULLVERSION)/$(TARNAME)-$(OSTYPE)-$(ARCH).tar.gz
ssh $(STAGINGSERVER) "touch nodejs/$(DISTTYPEDIR)/$(FULLVERSION)/$(TARNAME)-$(OSTYPE)-$(ARCH).tar.gz.done"
ifeq ($(XZ), 0)
ifeq ($(XZ), 1)
chmod 664 $(TARNAME)-$(OSTYPE)-$(ARCH).tar.xz
scp -p $(TARNAME)-$(OSTYPE)-$(ARCH).tar.xz $(STAGINGSERVER):nodejs/$(DISTTYPEDIR)/$(FULLVERSION)/$(TARNAME)-$(OSTYPE)-$(ARCH).tar.xz
ssh $(STAGINGSERVER) "touch nodejs/$(DISTTYPEDIR)/$(FULLVERSION)/$(TARNAME)-$(OSTYPE)-$(ARCH).tar.xz.done"
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.