rename bitcoin.conf in dist tarball #29903

pull willcl-ark wants to merge 1 commits into bitcoin:master from willcl-ark:conf-in-dst-share changing 1 files +1 −1

willcl-ark commented at 10:50 AM on April 18, 2024: member

Closes: #29139

Rename bitcoin.conf to bitcoin.conf.example in the guix dist tarball. This avoids confusion that the file is actively used by the binary in bin/ without being renamed.
DrahtBot commented at 10:50 AM on April 18, 2024: contributor



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



Code Coverage

For detailed information about the code coverage, see the test coverage report.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK josibake

Stale ACK laanwj

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
laanwj commented at 10:52 AM on April 18, 2024: member

Good idea, code review ACK 9256693be16cd56183557cf0bef57979fd04c39c
fanquake commented at 10:56 AM on April 18, 2024: member

Note previous discussion / change in #25935. cc @josibake
willcl-ark commented at 11:01 AM on April 18, 2024: member
Thanks @fanquake

it's more correct to have it in the root directory to 1) make it easier for the user to find it and 2) be clear that it is not an example config but a full config
1. IMO people can find it just fine in share/examples, in fact I'd be more likely to ls that dir looking for it personally...
2. Would be happy to meet @josibake halfway and classify it as a "full example config" 😄
laanwj commented at 11:07 AM on April 18, 2024: member
i think the point that it isn't actually used is a good one, and it would be good to make that clear, i see two options for that:
- Move it to directory called example (maybe better than share/example?)
- Put example in the file name, like bitcoin.conf.example
DrahtBot added the label CI failed on Apr 18, 2024
josibake commented at 12:42 PM on April 18, 2024: member
@laanwj @willcl-ark for context, the original goals of having this file were:
1. Make it clear that it was not an example, i.e. nothing in this file is pre-filled for the user, unlike our old bitcoin.conf which did have an example config which was often outdated
2. Make it easy to discover, to deter users from going to some third party for a bitcoin.conf generator (e.g. lopps website for example) or worse, getting a bitcoin.conf from "someone on the internet"
For 1, considering that we no longer provide an example config and it's fairly obvious when you open the file that it is a full config, I would prefer @laanwj 's suggestion of calling the file bitcoin.conf.example (I think all we would need to do then is update the docs where it prompts the user to copy the file to their datadir and mention it should be renamed to bitcoin.conf).

Not really a fan of moving the file into a different directory, since I think that makes the discoverability of the file slightly worse (which makes 2 more of a concern for me).
willcl-ark force-pushed on Apr 18, 2024
willcl-ark commented at 1:22 PM on April 18, 2024: member

Ok there seems to be some push back here. Personally, I'd prefer to see files neatly in directories so that folks installing from a tarball directly into /usr/local wouldn't have random odds-and-ends ending up in the root there, but happy to conceed on this (perhaps I install binary tarballs weirdly?)

Seems like the path of least resistance is just to append .example, so I've done that here now.

I think all we would need to do then is update the docs where it prompts the user to copy the file to their datadir and mention it should be renamed to bitcoin.conf

I don't believe we have such instructions for the tarball, only for source-generated configs, which we are not not going to match directory-wise, so leaving this suggestion for now.
josibake commented at 2:02 PM on April 18, 2024: member

Personally, I'd prefer to see files neatly in directories so that folks installing from a tarball directly into /usr/local wouldn't have random odds-and-ends ending up in the root there

We still have the README.md in the root, so moving this file doesn't really address that concern. If there is a compelling argument for having nothing in the root, then I'd argue config/bitcoin.conf.example would make the most sense (as this is what I've seen for most other tarball distributed software projects).
josibake commented at 2:05 PM on April 18, 2024: member

LGTM, although the commit message and PR title still mention moving to share/examples

Type	Reviewers
ACK	josibake
Stale ACK	laanwj

guix: rename bitcoin.conf in dist tarball

Motivated by #29139

Rename bitcoin.conf to bitcoin.conf.example in the guix dist tarball.
This avoids confusion that the file might be actively used by the
adjacent binaries in bin/ without being renamed.

7ee8680cfd

willcl-ark force-pushed on Apr 18, 2024
willcl-ark renamed this:
~~Move bitcoin.conf to /share/examples dir in releases~~
rename bitcoin.conf in dist tarball
on Apr 18, 2024
josibake commented at 7:52 AM on April 19, 2024: member

ACK https://github.com/bitcoin/bitcoin/pull/29903/commits/7ee8680cfd97d55b8dde8df5372fae5d9cef39fa
DrahtBot requested review from laanwj on Apr 19, 2024
pinheadmz commented at 5:09 PM on April 19, 2024: member

If we change the name of the file, I think we should also include a dummy-proof instruction to remove .example when deploying the file, here:

https://github.com/bitcoin/bitcoin/blob/67c0d93982ad214f5e0c9509e9dc3d6d792ad97c/contrib/devtools/gen-bitcoin-conf.sh#L40-L42
DrahtBot removed the label CI failed on Apr 23, 2024
willcl-ark commented at 9:29 AM on April 29, 2024: member

@josibake @laanwj I'm happy to close this (and the issue) based on discussion above and in agreement with @pinheadmz comment on the issue, if you both agree?
laanwj commented at 10:32 AM on April 29, 2024: member

Sure, if the outcome is "do nothing" that's optimal from our perspective. Enough serious issues...
josibake commented at 11:02 AM on April 29, 2024: member

Sure, if the outcome is "do nothing" that's optimal from our perspective. Enough serious issues...

+1.

Per #29903 (comment), it might make sense to revisit our tarball structure in the future and be in line with best practices for "installing by untarring into /usr/local", but I think that's a separate discussion so I'd suggest closing this for now.
willcl-ark closed this on Apr 29, 2024
bitcoin locked this on Apr 29, 2025

Contributors

Review Requested
laanwj

Linked (view graph)

#29139 ./bitcoin.conf file should not cause confusion with ./datadir/bitcoin.conf

rename bitcoin.conf in dist tarball #29903

Code Coverage

Reviews