Concept ACK.

Concept ACK.
Kicked a build to see if this works over gitian/depends build: https://bitcoin.jonasschnelli.ch/pulls/7603/

Looks like gitian is happy with this

@theuni I suppose this is ok with you?

I just rebased against latest master, and added a 2nd commit which removes the wxwidgets stuff as @laanwj and @MarcoFalke requested.

Ooh neat, I'm assigned. I guess I should take that as a hint and fix my mail filters so I'm not missing pings :)

Agree with @laanwj wrt avoiding hard-coding filenames. The names are already set in the top Makefile as @BITCOIN_QT_BIN@ and @BITCOIN_WIN_INSTALLER@, we just need to modify BITCOIN_QT_BIN to not include a path. I'll work up a quick change for that.

@JeremyRand Can you please take the top two commits from https://github.com/theuni/bitcoin/tree/7603 ? You can squash the NSIS one into your own. After that, I'm good with this.

We can't use BITCOIN_QT_BIN/BITCOIN_WIN_INSTALLER as I said above because they're not yet defined. I added new vars and used them in one place to ensure that everything stays synced up.

@theuni Is it okay if I modify the 4 lines at theuni@411a71f#diff-67e997bcfdac55191033d57a16d1408aR17 to use PACKAGE_TARNAME instead of hardcoding the "bitcoin" substring on those lines?

@JeremyRand I'd prefer not. The naming there should be the canonical definition, so it really shouldn't depend on anything else.

@theuni Hmm, in the use cases for changing these variables that I personally deal with (i.e. Namecoin) it's pretty much a given that when PACKAGE_TARNAME is changed, the binary filenames are going to change accordingly as well. Is there a use case I'm not seeing, where it's likely that PACKAGE_TARNAME and the prefix of the binary filenames will be different? (I admit that I'm not particularly familiar with the variety of customizations that people might make to the build scripts.)

Well the idea would be to keep as much flexibility as possible, so to be able to configure various names separately from each other.
I understand that you ideally want to keep your patch as small as possible, but if changing the executable names is at least centralized in one place that doesn't blow it up much.

@laanwj Yes, agreed that flexibility is important here. However, is there a loss in flexibility if a forked project has to change "$PACKAGE_TARNAME" to something else in the four binary name variables, rather than changing "bitcoin" to something else in the same variables? I'm definitely not arguing that those four variables should be removed, I just think it makes sense for their default values to be dependent on PACKAGE_TARNAME since that seems to be a common configuration. Any forked project that wants the binary filename prefixes to be something other than PACKAGE_TARNAME can change them just as easily as changing them away from being "bitcoin". So I don't see a loss of flexibility -- am I missing something?

The loss in flexibility appears when someone changes PACKAGE_TARNAME and is surprised all the binaries changed their name, too. So, has to go back and adjust the names of the binaries...

I think 644e6de is an uncontroversial improvement (after squash), which can be merged right now. If you really want to refactor PACKAGE_TARNAME, I'd prefer to do this in another pull.

I agree with @MarcoFalke.

@JeremyRand While it's obviously nice if our changes make life easier for other projects, our first priority has to be to our own.

There are plenty of scenarios where the 2 values could become out of sync, and forks may or may not wish to switch when we do. So inevitably, there's going to be some patching down the road. I'd rather not create something that appears to be a contract to downstreams in the process.

@theuni @MarcoFalke Okay, sounds good, you've convinced me. I'll add the suggested commits to this PR when I have a few minutes (I'm traveling today).

There we go. Hopefully I did the requested squash correctly.

utACK 0528e30

Looks much better now!
utACK 0528e30