build: remove GZIP export from gitian descriptors by fanquake · Pull Request #16141 · bitcoin/bitcoin

fanquake · 2019-06-03T20:01:44Z

The GZIP environment variable is deprecated, and everywhere that we invoke gzip we are already passing -9n directly, i.e:

  find bitcoin-* | sort | tar --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ../$SOURCEDIST

GZIP="-9n" gzip -h
gzip: warning: GZIP environment variable is deprecated; use an alias or script
Usage: gzip [OPTION]... [FILE]...

theuni · 2019-06-04T09:21:13Z

Loving these cleanups. utACK.

Since we call tar explicitly, maybe TAR_OPTIONS could be merged in and go away as well?

practicalswift · 2019-06-04T10:18:32Z

utACK 3ff1f2a

Agree with @theuni regarding TAR_OPTIONS. Parameters passed explicitly are easier to reason about.

DrahtBot · 2019-06-05T14:00:35Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#15569 (docs: improve linux tar packages by cisba)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

laanwj · 2019-06-06T10:42:39Z

contrib/gitian-descriptors/gitian-osx.yml

-  tar xf ../$SOURCEDIST
-  find bitcoin-* | sort | tar --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ../$SOURCEDIST
+  tar -xf ../$SOURCEDIST
+  find bitcoin-* | sort | tar --mtime="$REFERENCE_DATE\\\ $REFERENCE_TIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ../$SOURCEDIST


Wait, so TAR_OPTIONS environment variable is used by tar itself?

bc8863b depends: remove usage of TAR_OPTIONS (fanquake) 3ff1f2a build: remove export GZIP from gitian descriptors (fanquake) Pull request description: The `GZIP` environment variable is [deprecated](https://www.gnu.org/software/gzip/manual/gzip.html#Environment), and everywhere that we invoke `gzip` we are already passing `-9n` directly, i.e: ```base find bitcoin-* | sort | tar --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ../$SOURCEDIST ``` ```bash GZIP="-9n" gzip -h gzip: warning: GZIP environment variable is deprecated; use an alias or script Usage: gzip [OPTION]... [FILE]... ``` ACKs for commit bc8863: Tree-SHA512: 2d5277f7bf096fd5bd0dda47dfaf2dc7a31cc5d91eb8cb42db9cbe060d07dff66bf8e1122a89a3a1b597a3b39dbf8d9a8da4f02e642f58e451ce9fb24cc59769

maflcko · 2019-06-09T08:05:13Z

Why was this merged without a gitian build? Does the build pass for anyone?

maflcko · 2019-06-09T08:09:39Z

+ tar --strip-components=1 -xf ../bitcoin-0.18.99.tar.gz
tar: aclocal.m4: implausibly old time stamp -9223372036854775808
tar: autogen.sh: implausibly old time stamp -9223372036854775808
tar: build-aux/compile: implausibly old time stamp -9223372036854775808
tar: build-aux/config.guess: implausibly old time stamp -9223372036854775808
tar: build-aux/config.sub: implausibly old time stamp -9223372036854775808
tar: build-aux/depcomp: implausibly old time stamp -9223372036854775808

…scriptors" This reverts commit ef6b15c.

Fixes regression introduced by bitcoin#16141 -BEGIN VERIFY SCRIPT- sed -i 's#\$REFERENCE_DATE\\\\\\ \$REFERENCE_TIME#\$REFERENCE_DATETIME#g' contrib/gitian-descriptors/* -END VERIFY SCRIPT-

laanwj · 2019-06-10T21:03:28Z

~~Ok, whoops, let's revert this for now.~~ looks like there's already a fix that moves forward instead

fanquake · 2019-06-10T21:05:17Z

@laanwj I don't think we need to revert, there is a fix up in #16184.

993aa41 scripted-diff: gitian: Use REFERENCE_DATETIME directly. (Carl Dong) Pull request description: Fixes regression introduced by #16141. ``` -BEGIN VERIFY SCRIPT- sed -i 's#\$REFERENCE_DATE\\\\\\ \$REFERENCE_TIME#\$REFERENCE_DATETIME#g' contrib/gitian-descriptors/* -END VERIFY SCRIPT- ``` ----- Note that this could have been fixed by escaping properly, but using `REFERENCE_DATETIME` directly is simpler. Future note: `REFERENCE_{DATE{,DATETIME},TIME}` is a bit ridiculous. At the very _least_ gitian should use epoch, as it is the most parse-able, and preferably set SOURCE_DATE_EPOCH. ACKs for commit 993aa4: Tree-SHA512: 8457e5fffde66e1d2b846547b6807416b884c171f63569f76dfefd498d2a58ad6f9eb93931eb6cfc7ff38c6b460b0c488ca87d1a68bc630c48f365a74b6ee163

Fixes regression introduced by bitcoin#16141 -BEGIN VERIFY SCRIPT- sed -i 's#\$REFERENCE_DATE\\\\\\ \$REFERENCE_TIME#\$REFERENCE_DATETIME#g' contrib/gitian-descriptors/* -END VERIFY SCRIPT-

…ptors Summary: bc8863b81922eb878519f328e9b0c7974aaa34ff depends: remove usage of TAR_OPTIONS (fanquake) 3ff1f2a319fc619954736d1e540ccbebc818ff11 build: remove export GZIP from gitian descriptors (fanquake) Pull request description: The `GZIP` environment variable is [deprecated](https://www.gnu.org/software/gzip/manual/gzip.html#Environment), and everywhere that we invoke `gzip` we are already passing `-9n` directly, i.e: ```base find bitcoin-* | sort | tar --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ../$SOURCEDIST ``` ```bash GZIP="-9n" gzip -h gzip: warning: GZIP environment variable is deprecated; use an alias or script Usage: gzip [OPTION]... [FILE]... ``` Merge #16184: scripted-diff: gitian: Use REFERENCE_DATETIME directly. Pull request description: Fixes regression introduced by #16141. ``` -BEGIN VERIFY SCRIPT- sed -i 's#\$REFERENCE_DATE\\\\\\ \$REFERENCE_TIME#\${REFERENCE_DATETIME}#g' contrib/gitian-descriptors/* -END VERIFY SCRIPT- ``` ----- Note that this could have been fixed by escaping properly, but using `REFERENCE_DATETIME` directly is simpler. Future note: `REFERENCE_{DATE{,DATETIME},TIME}` is a bit ridiculous. At the very _least_ gitian should use epoch, as it is the most parse-able, and preferably set SOURCE_DATE_EPOCH. Backport of Core [[bitcoin/bitcoin#16141 | PR16141]] and [[bitcoin/bitcoin#16184 | PR16184]] (to fix a regression in 16141) Test Plan: Run all Gitian builds in CI Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Subscribers: Fabien Differential Revision: https://reviews.bitcoinabc.org/D8136

Fixes regression introduced by bitcoin#16141 -BEGIN VERIFY SCRIPT- sed -i 's#\$REFERENCE_DATE\\\\\\ \$REFERENCE_TIME#\$REFERENCE_DATETIME#g' contrib/gitian-descriptors/* -END VERIFY SCRIPT-

…scriptors a7dbb27 build: add missing attributes to Win installer (Fuzzbawls) 13d0b48 nsis: Don't duplicate URL string (Fuzzbawls) 56563e5 build: remove WINDOWS_BITS from build system (fanquake) 0cb37c8 nsis: Write to correct filename in first place (Carl Dong) 7aa2823 gitian: Put things in the right place to begin with (Carl Dong) 0dcf6e6 gitian: Eliminate rename dependency (Carl Dong) ba579b9 gitian: Smaller diff with gitian-linux.yml (Carl Dong) 3b9d5e8 gitian: Fix README inclusion in archives (Carl Dong) 07c27d2 gitian: Use split-debug.sh for Win builds (Carl Dong) 9f19095 build: Add README.md to DIST target (MarcoFalke) ce80c76 build: remove mingw linker workaround from win gitian descriptor (fanquake) 37441b0 scripted-diff: gitian: Use REFERENCE_DATETIME directly. (Fuzzbawls) 966fe9a depends: remove usage of TAR_OPTIONS (fanquake) 12fd641 build: remove export GZIP from gitian descriptors (fanquake) Pull request description: This is a collection of upstream PRs focused on cleaning up the gitian descriptors (un-needed or duplicate code bits, standardizing similar actions across the descriptors, etc), and minor cleanups to supporting build system files. Included are the following upstream PRs: * bitcoin#16141 * bitcoin#16184 * bitcoin#16667 * bitcoin#17097 * bitcoin#17029 * bitcoin#17308 * bitcoin#17756 * bitcoin#18059 ACKs for top commit: furszy: Tested ACK a7dbb27 random-zebra: ACK a7dbb27 and merging... Tree-SHA512: 409d4f51adfb1c90aa8bc572fe6610596dea1293a8ca795f9071bb88bd583bd7678359ee19e34440bb0d117075ebd27f9a622247ee3dfd0cd9088e6da306a4d5

bc8863b depends: remove usage of TAR_OPTIONS (fanquake) 3ff1f2a build: remove export GZIP from gitian descriptors (fanquake) Pull request description: The `GZIP` environment variable is [deprecated](https://www.gnu.org/software/gzip/manual/gzip.html#Environment), and everywhere that we invoke `gzip` we are already passing `-9n` directly, i.e: ```base find bitcoin-* | sort | tar --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ../$SOURCEDIST ``` ```bash GZIP="-9n" gzip -h gzip: warning: GZIP environment variable is deprecated; use an alias or script Usage: gzip [OPTION]... [FILE]... ``` ACKs for commit bc8863: Tree-SHA512: 2d5277f7bf096fd5bd0dda47dfaf2dc7a31cc5d91eb8cb42db9cbe060d07dff66bf8e1122a89a3a1b597a3b39dbf8d9a8da4f02e642f58e451ce9fb24cc59769

…rectly. 993aa41 scripted-diff: gitian: Use REFERENCE_DATETIME directly. (Carl Dong) Pull request description: Fixes regression introduced by bitcoin#16141. ``` -BEGIN VERIFY SCRIPT- sed -i 's#\$REFERENCE_DATE\\\\\\ \$REFERENCE_TIME#\$REFERENCE_DATETIME#g' contrib/gitian-descriptors/* -END VERIFY SCRIPT- ``` ----- Note that this could have been fixed by escaping properly, but using `REFERENCE_DATETIME` directly is simpler. Future note: `REFERENCE_{DATE{,DATETIME},TIME}` is a bit ridiculous. At the very _least_ gitian should use epoch, as it is the most parse-able, and preferably set SOURCE_DATE_EPOCH. ACKs for commit 993aa4: Tree-SHA512: 8457e5fffde66e1d2b846547b6807416b884c171f63569f76dfefd498d2a58ad6f9eb93931eb6cfc7ff38c6b460b0c488ca87d1a68bc630c48f365a74b6ee163

bc8863b depends: remove usage of TAR_OPTIONS (fanquake) 3ff1f2a build: remove export GZIP from gitian descriptors (fanquake) Pull request description: The `GZIP` environment variable is [deprecated](https://www.gnu.org/software/gzip/manual/gzip.html#Environment), and everywhere that we invoke `gzip` we are already passing `-9n` directly, i.e: ```base find bitcoin-* | sort | tar --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ../$SOURCEDIST ``` ```bash GZIP="-9n" gzip -h gzip: warning: GZIP environment variable is deprecated; use an alias or script Usage: gzip [OPTION]... [FILE]... ``` ACKs for commit bc8863: Tree-SHA512: 2d5277f7bf096fd5bd0dda47dfaf2dc7a31cc5d91eb8cb42db9cbe060d07dff66bf8e1122a89a3a1b597a3b39dbf8d9a8da4f02e642f58e451ce9fb24cc59769

…rectly. 993aa41 scripted-diff: gitian: Use REFERENCE_DATETIME directly. (Carl Dong) Pull request description: Fixes regression introduced by bitcoin#16141. ``` -BEGIN VERIFY SCRIPT- sed -i 's#\$REFERENCE_DATE\\\\\\ \$REFERENCE_TIME#\$REFERENCE_DATETIME#g' contrib/gitian-descriptors/* -END VERIFY SCRIPT- ``` ----- Note that this could have been fixed by escaping properly, but using `REFERENCE_DATETIME` directly is simpler. Future note: `REFERENCE_{DATE{,DATETIME},TIME}` is a bit ridiculous. At the very _least_ gitian should use epoch, as it is the most parse-able, and preferably set SOURCE_DATE_EPOCH. ACKs for commit 993aa4: Tree-SHA512: 8457e5fffde66e1d2b846547b6807416b884c171f63569f76dfefd498d2a58ad6f9eb93931eb6cfc7ff38c6b460b0c488ca87d1a68bc630c48f365a74b6ee163

build: remove export GZIP from gitian descriptors

3ff1f2a

fanquake added Build system Needs gitian build labels Jun 3, 2019

fanquake requested a review from dongcarl June 3, 2019 20:01

depends: remove usage of TAR_OPTIONS

bc8863b

laanwj reviewed Jun 6, 2019

View reviewed changes

laanwj merged commit bc8863b into bitcoin:master Jun 7, 2019

maflcko removed the Needs gitian build label Jun 7, 2019

fanquake deleted the unused_gzip branch June 7, 2019 13:42

laanwj mentioned this pull request Jun 9, 2019

contrib: Enable building in Guix containers #15277

Merged

18 tasks

sidhujag added a commit to syscoin/syscoin that referenced this pull request Jun 10, 2019

Revert "Merge bitcoin#16141: build: remove GZIP export from gitian de…

e014f99

…scriptors" This reverts commit ef6b15c.

dongcarl mentioned this pull request Jun 10, 2019

scripted-diff: gitian: Use REFERENCE_DATETIME directly. #16184

Merged

Fuzzbawls mentioned this pull request Jun 24, 2021

[Build][Cleanup] Tidy up the gitian build descriptors PIVX-Project/PIVX#2447

Merged

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: remove GZIP export from gitian descriptors#16141

build: remove GZIP export from gitian descriptors#16141
laanwj merged 2 commits intobitcoin:masterfrom
fanquake:unused_gzip

fanquake commented Jun 3, 2019

Uh oh!

theuni commented Jun 4, 2019

Uh oh!

practicalswift commented Jun 4, 2019

Uh oh!

DrahtBot commented Jun 5, 2019

Uh oh!

laanwj Jun 6, 2019

Uh oh!

maflcko commented Jun 9, 2019

Uh oh!

maflcko commented Jun 9, 2019

Uh oh!

laanwj commented Jun 10, 2019 •

edited

Loading

Uh oh!

fanquake commented Jun 10, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

fanquake commented Jun 3, 2019

Uh oh!

theuni commented Jun 4, 2019

Uh oh!

practicalswift commented Jun 4, 2019

Uh oh!

DrahtBot commented Jun 5, 2019

Conflicts

Uh oh!

laanwj Jun 6, 2019

Choose a reason for hiding this comment

Uh oh!

maflcko commented Jun 9, 2019

Uh oh!

maflcko commented Jun 9, 2019

Uh oh!

laanwj commented Jun 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fanquake commented Jun 10, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

laanwj commented Jun 10, 2019 •

edited

Loading