Skip to content

Conversation

ch4ot1c
Copy link
Contributor

@ch4ot1c ch4ot1c commented Oct 10, 2019

Changes make dist to use git archive instead of its default approach ,EXTRA_DIST.

The clientversion.cpp version suffix is set to the commit hash of HEAD. There are additional files that are appended beyond this, namely configure and other autotools generated files. The .git directory is not included. Target dist: is used without a dist-hook:.

./autogen.sh && ./configure && make dist

Lastly, simplifies the layout of Makefile.am.

Note - MacOS would need gtar to use tar --transform for appending the autotools generated files in a prefixed way (an alternate approach). This PR uses git add -f instead.

@luke-jr
Copy link
Member

luke-jr commented Oct 10, 2019

Concept ACK, but the distfile needs to include autogen-generated files.

And I don't see why it should be running submodules' make dists...

@ch4ot1c
Copy link
Contributor Author

ch4ot1c commented Oct 10, 2019

I agree, for GNU conformance and support for exotic systems.

Moved the univalue issue+pr to its upstream repo.

@DrahtBot
Copy link
Contributor

DrahtBot commented Oct 11, 2019

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #17091 (tests: Add test for loadblock option and linearize scripts by fjahr)

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.

@ch4ot1c ch4ot1c force-pushed the build/archive-dist branch 2 times, most recently from 1c71045 to 2ab4c3a Compare October 20, 2019 14:22
- Remove `EXTRA_DIST` entries
- Delineate MacOS and Windows sections
- Use `WIN_` prefix consistently
@ch4ot1c ch4ot1c force-pushed the build/archive-dist branch from 2ab4c3a to fa06c83 Compare October 20, 2019 14:31
Keeps its behavior:

- Sets version in src/clientversion.cpp to HEAD commit
- Adds autotools-generated files like `configure`
@ch4ot1c ch4ot1c force-pushed the build/archive-dist branch from fa06c83 to 6e7ef96 Compare October 20, 2019 14:48
@ch4ot1c ch4ot1c changed the title [WIP] build: make dist uses git archive build: make dist uses git archive Oct 20, 2019
@DrahtBot
Copy link
Contributor

Needs rebase

@laanwj laanwj added this to the 0.20.0 milestone Nov 18, 2019
@laanwj
Copy link
Member

laanwj commented Nov 18, 2019

Would be nice to have this in 0.20.0.

@hebasto
Copy link
Member

hebasto commented Mar 10, 2020

@ch4ot1c Are you still working on this?

@laanwj
Copy link
Member

laanwj commented Mar 11, 2020

Labeling this "up for grabs". It'd be nice to have this for 0.20.

laanwj added a commit that referenced this pull request Mar 25, 2020
e4d3667 build: Drop needless EXTRA_DIST content (Hennadii Stepanov)
6c4da59 build: Drop SOURCEDIST reordering (Hennadii Stepanov)
5e6b8b3 build: Use git archive as source tarball (Hennadii Stepanov)

Pull request description:

  This PR:
  - is an alternative to #17104
  - closes #16734
  - closes #6753

  The idea is clear described by some developers:
  - [MarcoFalke](#17097 (comment)):
  > This whole concept of explicitly listing each and every file manually (or with a fragile wildcard) is an obvious sisyphean task. I'd say all we need to do is run git archive and be done with it forever, see #16734, #6753, #11530 ...

  - [laanwj](#17097 (comment)):
  > I agree, I've never been a fan of it. I don't think we have any files in the git repository we don't want to ship in the source tarball.

  ---

  The suggested changes have a downside which is pointed by [**luke-jr**](#17104 (comment)):
  > ... but the distfile needs to include autogen-generated files.

  This means that a user is not able to run `./configure && make` right away. One must run `./autogen.sh` at first.

  Here are opinions about mandatory use of `./autogen.sh`:
  - [ryanofsky](#16734 (comment)):
  > It's probably ok to require autogen. I think historically configure scripts were supposed to work on obscure unix systems that would just have a generic shell + make tool + c compiler, and not necessarily need gnu packages like m4 which are needed for autogen.

  - [laanwj](#16734 (comment)):
  > I also think it's fine to require autogen. What is one dependency more, if you're building from source.

  ---

  ~Also this PR provides Windows users with ZIP archives of the sources. Additionally the commit ID is stored in these ZIP files as a file comment:~

  ---

  Note for reviewers: please verify is `git archive` output deterministic?

ACKs for top commit:
  MarcoFalke:
    re-ACK e4d3667, only change is adding two dots in a the path 🛳
  laanwj:
    ACK e4d3667

Tree-SHA512: d1153d3ca4a580696019b92be3555ab004d197d9a2146aacff9d3150eb7093b7d40eebd6eea12d861d93ff62d62b68706e04e64dbe5ea796ff6757486e462193
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Mar 28, 2020
e4d3667 build: Drop needless EXTRA_DIST content (Hennadii Stepanov)
6c4da59 build: Drop SOURCEDIST reordering (Hennadii Stepanov)
5e6b8b3 build: Use git archive as source tarball (Hennadii Stepanov)

Pull request description:

  This PR:
  - is an alternative to bitcoin#17104
  - closes bitcoin#16734
  - closes bitcoin#6753

  The idea is clear described by some developers:
  - [MarcoFalke](bitcoin#17097 (comment)):
  > This whole concept of explicitly listing each and every file manually (or with a fragile wildcard) is an obvious sisyphean task. I'd say all we need to do is run git archive and be done with it forever, see bitcoin#16734, bitcoin#6753, bitcoin#11530 ...

  - [laanwj](bitcoin#17097 (comment)):
  > I agree, I've never been a fan of it. I don't think we have any files in the git repository we don't want to ship in the source tarball.

  ---

  The suggested changes have a downside which is pointed by [**luke-jr**](bitcoin#17104 (comment)):
  > ... but the distfile needs to include autogen-generated files.

  This means that a user is not able to run `./configure && make` right away. One must run `./autogen.sh` at first.

  Here are opinions about mandatory use of `./autogen.sh`:
  - [ryanofsky](bitcoin#16734 (comment)):
  > It's probably ok to require autogen. I think historically configure scripts were supposed to work on obscure unix systems that would just have a generic shell + make tool + c compiler, and not necessarily need gnu packages like m4 which are needed for autogen.

  - [laanwj](bitcoin#16734 (comment)):
  > I also think it's fine to require autogen. What is one dependency more, if you're building from source.

  ---

  ~Also this PR provides Windows users with ZIP archives of the sources. Additionally the commit ID is stored in these ZIP files as a file comment:~

  ---

  Note for reviewers: please verify is `git archive` output deterministic?

ACKs for top commit:
  MarcoFalke:
    re-ACK e4d3667, only change is adding two dots in a the path 🛳
  laanwj:
    ACK e4d3667

Tree-SHA512: d1153d3ca4a580696019b92be3555ab004d197d9a2146aacff9d3150eb7093b7d40eebd6eea12d861d93ff62d62b68706e04e64dbe5ea796ff6757486e462193
UdjinM6 pushed a commit to UdjinM6/dash that referenced this pull request Oct 23, 2021
e4d3667 build: Drop needless EXTRA_DIST content (Hennadii Stepanov)
6c4da59 build: Drop SOURCEDIST reordering (Hennadii Stepanov)
5e6b8b3 build: Use git archive as source tarball (Hennadii Stepanov)

Pull request description:

  This PR:
  - is an alternative to bitcoin#17104
  - closes bitcoin#16734
  - closes dashpay#6753

  The idea is clear described by some developers:
  - [MarcoFalke](bitcoin#17097 (comment)):
  > This whole concept of explicitly listing each and every file manually (or with a fragile wildcard) is an obvious sisyphean task. I'd say all we need to do is run git archive and be done with it forever, see bitcoin#16734, dashpay#6753, bitcoin#11530 ...

  - [laanwj](bitcoin#17097 (comment)):
  > I agree, I've never been a fan of it. I don't think we have any files in the git repository we don't want to ship in the source tarball.

  ---

  The suggested changes have a downside which is pointed by [**luke-jr**](bitcoin#17104 (comment)):
  > ... but the distfile needs to include autogen-generated files.

  This means that a user is not able to run `./configure && make` right away. One must run `./autogen.sh` at first.

  Here are opinions about mandatory use of `./autogen.sh`:
  - [ryanofsky](bitcoin#16734 (comment)):
  > It's probably ok to require autogen. I think historically configure scripts were supposed to work on obscure unix systems that would just have a generic shell + make tool + c compiler, and not necessarily need gnu packages like m4 which are needed for autogen.

  - [laanwj](bitcoin#16734 (comment)):
  > I also think it's fine to require autogen. What is one dependency more, if you're building from source.

  ---

  ~Also this PR provides Windows users with ZIP archives of the sources. Additionally the commit ID is stored in these ZIP files as a file comment:~

  ---

  Note for reviewers: please verify is `git archive` output deterministic?

ACKs for top commit:
  MarcoFalke:
    re-ACK e4d3667, only change is adding two dots in a the path 🛳
  laanwj:
    ACK e4d3667

Tree-SHA512: d1153d3ca4a580696019b92be3555ab004d197d9a2146aacff9d3150eb7093b7d40eebd6eea12d861d93ff62d62b68706e04e64dbe5ea796ff6757486e462193
UdjinM6 pushed a commit to UdjinM6/dash that referenced this pull request Oct 23, 2021
e4d3667 build: Drop needless EXTRA_DIST content (Hennadii Stepanov)
6c4da59 build: Drop SOURCEDIST reordering (Hennadii Stepanov)
5e6b8b3 build: Use git archive as source tarball (Hennadii Stepanov)

Pull request description:

  This PR:
  - is an alternative to bitcoin#17104
  - closes bitcoin#16734
  - closes dashpay#6753

  The idea is clear described by some developers:
  - [MarcoFalke](bitcoin#17097 (comment)):
  > This whole concept of explicitly listing each and every file manually (or with a fragile wildcard) is an obvious sisyphean task. I'd say all we need to do is run git archive and be done with it forever, see bitcoin#16734, dashpay#6753, bitcoin#11530 ...

  - [laanwj](bitcoin#17097 (comment)):
  > I agree, I've never been a fan of it. I don't think we have any files in the git repository we don't want to ship in the source tarball.

  ---

  The suggested changes have a downside which is pointed by [**luke-jr**](bitcoin#17104 (comment)):
  > ... but the distfile needs to include autogen-generated files.

  This means that a user is not able to run `./configure && make` right away. One must run `./autogen.sh` at first.

  Here are opinions about mandatory use of `./autogen.sh`:
  - [ryanofsky](bitcoin#16734 (comment)):
  > It's probably ok to require autogen. I think historically configure scripts were supposed to work on obscure unix systems that would just have a generic shell + make tool + c compiler, and not necessarily need gnu packages like m4 which are needed for autogen.

  - [laanwj](bitcoin#16734 (comment)):
  > I also think it's fine to require autogen. What is one dependency more, if you're building from source.

  ---

  ~Also this PR provides Windows users with ZIP archives of the sources. Additionally the commit ID is stored in these ZIP files as a file comment:~

  ---

  Note for reviewers: please verify is `git archive` output deterministic?

ACKs for top commit:
  MarcoFalke:
    re-ACK e4d3667, only change is adding two dots in a the path 🛳
  laanwj:
    ACK e4d3667

Tree-SHA512: d1153d3ca4a580696019b92be3555ab004d197d9a2146aacff9d3150eb7093b7d40eebd6eea12d861d93ff62d62b68706e04e64dbe5ea796ff6757486e462193
UdjinM6 pushed a commit to UdjinM6/dash that referenced this pull request Dec 4, 2021
e4d3667 build: Drop needless EXTRA_DIST content (Hennadii Stepanov)
6c4da59 build: Drop SOURCEDIST reordering (Hennadii Stepanov)
5e6b8b3 build: Use git archive as source tarball (Hennadii Stepanov)

Pull request description:

  This PR:
  - is an alternative to bitcoin#17104
  - closes bitcoin#16734
  - closes dashpay#6753

  The idea is clear described by some developers:
  - [MarcoFalke](bitcoin#17097 (comment)):
  > This whole concept of explicitly listing each and every file manually (or with a fragile wildcard) is an obvious sisyphean task. I'd say all we need to do is run git archive and be done with it forever, see bitcoin#16734, dashpay#6753, bitcoin#11530 ...

  - [laanwj](bitcoin#17097 (comment)):
  > I agree, I've never been a fan of it. I don't think we have any files in the git repository we don't want to ship in the source tarball.

  ---

  The suggested changes have a downside which is pointed by [**luke-jr**](bitcoin#17104 (comment)):
  > ... but the distfile needs to include autogen-generated files.

  This means that a user is not able to run `./configure && make` right away. One must run `./autogen.sh` at first.

  Here are opinions about mandatory use of `./autogen.sh`:
  - [ryanofsky](bitcoin#16734 (comment)):
  > It's probably ok to require autogen. I think historically configure scripts were supposed to work on obscure unix systems that would just have a generic shell + make tool + c compiler, and not necessarily need gnu packages like m4 which are needed for autogen.

  - [laanwj](bitcoin#16734 (comment)):
  > I also think it's fine to require autogen. What is one dependency more, if you're building from source.

  ---

  ~Also this PR provides Windows users with ZIP archives of the sources. Additionally the commit ID is stored in these ZIP files as a file comment:~

  ---

  Note for reviewers: please verify is `git archive` output deterministic?

ACKs for top commit:
  MarcoFalke:
    re-ACK e4d3667, only change is adding two dots in a the path 🛳
  laanwj:
    ACK e4d3667

Tree-SHA512: d1153d3ca4a580696019b92be3555ab004d197d9a2146aacff9d3150eb7093b7d40eebd6eea12d861d93ff62d62b68706e04e64dbe5ea796ff6757486e462193
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants