Skip to content

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Oct 19, 2017

As the legacy rpcuser and rpcpassword are deprected since 0.12.0, we should actually include the script to generate the new auth pair in the distributed source code archive.

Ref: #6753

(Tagging for backport, since it is a trivial bugfix)

@maflcko maflcko added this to the 0.15.1 milestone Oct 19, 2017
@laanwj
Copy link
Member

laanwj commented Oct 21, 2017

This is certainly a step forward, but if we expect everyone to use it, shouldn't we include it in binary distributions as well - or even just install it as a command in make install?

(I'm ambivalent on removing rpcuser/rpcpassword btw, I think they're useful enough to keep around for simple and quick authentication configuration, but that discussion doesn't need to be here)

Copy link
Contributor

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

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

utACK fa81534

Agree with laanw that it probably also does make sense to install, too. But that change will require this one.

@laanwj
Copy link
Member

laanwj commented Oct 26, 2017

Yes, this is a step forward in any case... utACK

@laanwj laanwj merged commit fa81534 into bitcoin:master Oct 26, 2017
laanwj added a commit that referenced this pull request Oct 26, 2017
fa81534 Add share/rpcuser to dist. source code archive (MarcoFalke)

Pull request description:

  As the legacy rpcuser and rpcpassword are deprected since 0.12.0, we should actually include the script to generate the new auth pair in the distributed source code archive.

  Ref: #6753

  (Tagging for backport, since it is a trivial bugfix)

Tree-SHA512: f2737957a92396444573f41071a785be5fb318df9efeb3ade7e56b3b56d512e5f9ca36723365fe5be8aaee69c5e8d8ed1178510bf02186c848b3910ee001ecb9
laanwj pushed a commit that referenced this pull request Oct 26, 2017
Github-Pull: #11530
Rebased-From: fa81534
Tree-SHA512: 97bc91760a3284a9b60dcde04e789aed3c83539ec6621cb38dbc5bd852bfc0cdcaffceff7ca6de0c64f00149e6774d7dd651520b39876a674f1e82efba98945d
@NicolasDorier
Copy link
Contributor

Please do not remove user/pwd until bitcoin-cli can generate the creds.
Having to install python just for generating credentials is a PITA, probably the main reason why people are still using usr/pwd.

@maflcko maflcko deleted the Mf1710-distShare branch October 26, 2017 18:16
@maflcko
Copy link
Member Author

maflcko commented Oct 26, 2017

@NicolasDorier No worries. No one created a pull for it, so I don't think that will happen any time soon.

codablock pushed a commit to codablock/dash that referenced this pull request Sep 26, 2019
fa81534 Add share/rpcuser to dist. source code archive (MarcoFalke)

Pull request description:

  As the legacy rpcuser and rpcpassword are deprected since 0.12.0, we should actually include the script to generate the new auth pair in the distributed source code archive.

  Ref: dashpay#6753

  (Tagging for backport, since it is a trivial bugfix)

Tree-SHA512: f2737957a92396444573f41071a785be5fb318df9efeb3ade7e56b3b56d512e5f9ca36723365fe5be8aaee69c5e8d8ed1178510bf02186c848b3910ee001ecb9
codablock pushed a commit to codablock/dash that referenced this pull request Sep 30, 2019
fa81534 Add share/rpcuser to dist. source code archive (MarcoFalke)

Pull request description:

  As the legacy rpcuser and rpcpassword are deprected since 0.12.0, we should actually include the script to generate the new auth pair in the distributed source code archive.

  Ref: dashpay#6753

  (Tagging for backport, since it is a trivial bugfix)

Tree-SHA512: f2737957a92396444573f41071a785be5fb318df9efeb3ade7e56b3b56d512e5f9ca36723365fe5be8aaee69c5e8d8ed1178510bf02186c848b3910ee001ecb9
barrystyle pushed a commit to PACGlobalOfficial/PAC that referenced this pull request Jan 22, 2020
fa81534 Add share/rpcuser to dist. source code archive (MarcoFalke)

Pull request description:

  As the legacy rpcuser and rpcpassword are deprected since 0.12.0, we should actually include the script to generate the new auth pair in the distributed source code archive.

  Ref: dashpay#6753

  (Tagging for backport, since it is a trivial bugfix)

Tree-SHA512: f2737957a92396444573f41071a785be5fb318df9efeb3ade7e56b3b56d512e5f9ca36723365fe5be8aaee69c5e8d8ed1178510bf02186c848b3910ee001ecb9
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
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
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.

4 participants