[WIP] depends: Add native_nsis to support unicode #13827
Conversation
|
Concept ACK Looks like the |
Reviewers, this pull request conflicts with the following ones:
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. |
|
Since this is not a regression, I have assigned the 0.18.0 milestone for now. |
| define $(package)_stage_cmds | ||
| $(MAKE) install prefix=$($(package)_staging_prefix_dir) | ||
| endef | ||
|
|
There was a problem hiding this comment.
Please use "linux encoding" for the "new line characters"
|
Crap, one of the reasons we upgraded to bionic in the first place is to upgrade NSIS, not really happy that we end up compiling it anyway now. |
depends/packages/native_nsis.mk
Outdated
| $(package)_dependencies=native_zlib | ||
|
|
||
| define $(package)_set_vars | ||
| $(package)_build_opts = -j4 |
There was a problem hiding this comment.
I'm not sure what the best number is here.
There was a problem hiding this comment.
Can't you use the passed down number from make depends -j yy?
There was a problem hiding this comment.
nsis is not built by make, so it is incompatible with make jobs.
There was a problem hiding this comment.
Or there is a way to get job number?
There was a problem hiding this comment.
Not that I am aware of. Other than that you might just set the number of jobs so that they use less than 1GB of RAM, since that is our minimum requirement to get built.
There was a problem hiding this comment.
After testing a few times, there are some edge case fail to cd directories in multi-job. Will remove this.
|
@MarcoFalke This is not ready for gitian build since it is not deterministic. |
|
I have no idea why the following things would happen? I already put scons into faketime wrapper, but the compiled stub files still contain timestamp in PE header. The alternative way is to hack the compiled PE file timestamp and set it to a constant. |
|
@theuni Do you have any idea what would break the deterministic? |
|
Maybe it helps if you include the |
|
Yes, I know it contains current timestamp and checksum, that is the only differnece. But I don't know why it would be there. I already pack scons into faketime wrapper. |
|
The difference is here: Apparently it is timestamp. |
|
Could you also upload both of the tar.gz results? |
|
Close it for now. I can't solve the gitian build deterministic issue. |
|
Oh, wait. Why is it required that the depends cache is deterministic? Shouldn't the only requirement be that the resulting bitcoind.exe (et al) is deterministic? If you reopen, I can run DrahtBot on this twice to see what the effective difference is for Gitian builds. |
It has to compile installer and decompressor that runs on Windows.
Thanks, but you have to clear nsis depends cache manually, otherwise it's deterministic. |
| Needs rebase |
|
Does this upstream patch fix this? https://sourceforge.net/p/nsis/bugs/1230/ |
|
Removing "Up for grabs", as this should be taken care of by #21333. |
Since version 3.0, nsis start to support unicode. However, Bionic still use 2.51. So, it might be a good reason to add nsis into dependency list to support unicode.
Fix #13817
Close #8825
TODO: