[WIP] depends: Add native_nsis to support unicode

ken2812221 commented at 1:35 pm on August 1, 2018: contributor

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:

Gitian build should be deterministic

fanquake added the label Build system on Aug 1, 2018

fanquake commented at 1:38 pm on August 1, 2018: member

Concept ACK This and Unicode true was what I was looking into in regards to #13817.

Looks like the whitespace linter is unhappy.

DrahtBot commented at 1:58 pm on August 1, 2018: member

#13515 (travis: Enable qt for all jobs by ken2812221)

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.

MarcoFalke added this to the milestone 0.18.0 on Aug 1, 2018

MarcoFalke commented at 2:03 pm on August 1, 2018: member

Since this is not a regression, I have assigned the 0.18.0 milestone for now.

ken2812221 force-pushed on Aug 1, 2018

in depends/packages/native_zlib_posix.mk:18 in 33245565fc outdated

13+endef
14+
15+define $(package)_stage_cmds
16+  $(MAKE) install prefix=$($(package)_staging_prefix_dir)
17+endef
18+

MarcoFalke commented at 2:05 pm on August 1, 2018:

Please use “linux encoding” for the “new line characters”

ken2812221 force-pushed on Aug 1, 2018

laanwj commented at 3:29 pm on August 1, 2018: member

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. But yes, let’s put this for 0.18, this is not a new problem (and comes extremely last-minute for 0.17).

ken2812221 force-pushed on Aug 1, 2018

ken2812221 force-pushed on Aug 2, 2018

ken2812221 renamed this:
~~depends: Add native_nsis to support unicode~~
wip, depends: Add native_nsis to support unicode
on Aug 2, 2018

ken2812221 force-pushed on Aug 2, 2018

ken2812221 renamed this:
~~wip, depends: Add native_nsis to support unicode~~
depends: Add native_nsis to support unicode
on Aug 2, 2018

in depends/packages/native_nsis.mk:9 in b0753d1dc8 outdated

0@@ -0,0 +1,27 @@
1+package=native_nsis
2+$(package)_version=3.03
3+$(package)_download_path=https://sourceforge.net/projects/nsis/files/NSIS%203/$($(package)_version)
4+$(package)_file_name=nsis-$($(package)_version)-src.tar.bz2
5+$(package)_sha256_hash=abae7f4488bc6de7a4dd760d5f0e7cd3aad7747d4d7cd85786697c8991695eaa
6+$(package)_dependencies=native_zlib
7+
8+define $(package)_set_vars
9+$(package)_build_opts  = -j4

ken2812221 commented at 11:54 am on August 2, 2018:

I’m not sure what the best number is here.

MarcoFalke commented at 11:56 am on August 2, 2018:

Can’t you use the passed down number from make depends -j yy?

ken2812221 commented at 11:59 am on August 2, 2018:

nsis is not built by make, so it is incompatible with make jobs.

ken2812221 commented at 12:00 pm on August 2, 2018:

Or there is a way to get job number?

MarcoFalke commented at 12:08 pm on August 2, 2018:

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.

ken2812221 commented at 7:55 am on August 6, 2018:

After testing a few times, there are some edge case fail to cd directories in multi-job. Will remove this.

ken2812221 force-pushed on Aug 6, 2018

ken2812221 renamed this:
~~depends: Add native_nsis to support unicode~~
wip, depends: Add native_nsis to support unicode
on Aug 7, 2018

laanwj added the label Windows on Aug 7, 2018

MarcoFalke added the label Needs gitian build on Aug 7, 2018

ken2812221 commented at 6:15 pm on August 7, 2018: contributor

@MarcoFalke This is not ready for gitian build since it is not deterministic.

DrahtBot removed the label Needs gitian build on Aug 8, 2018

ken2812221 force-pushed on Aug 15, 2018

ken2812221 force-pushed on Aug 16, 2018

ken2812221 force-pushed on Aug 17, 2018

ken2812221 commented at 10:16 am on August 22, 2018: contributor

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.

MarcoFalke deleted a comment on Aug 22, 2018

ken2812221 force-pushed on Aug 22, 2018

DrahtBot added the label Needs rebase on Aug 27, 2018

ken2812221 force-pushed on Aug 29, 2018

depends: native_nsis 42cc79bc1d

ken2812221 force-pushed on Aug 29, 2018

DrahtBot removed the label Needs rebase on Aug 29, 2018

ken2812221 commented at 7:53 pm on August 30, 2018: contributor

@theuni Do you have any idea what would break the deterministic?

MarcoFalke commented at 7:55 pm on August 30, 2018: member

Maybe it helps if you include the diffoscope output between two consecutive builds?

ken2812221 commented at 7:58 pm on August 30, 2018: contributor

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.

ken2812221 commented at 10:06 am on August 31, 2018: contributor

The difference is here: https://0bin.net/paste/Z5FvTgDDCLs2El3y#fvLayra1H9pE-R0lc/QX026USa0ZYn5PgTWKDCFqkT5

Apparently it is timestamp.

MarcoFalke commented at 1:18 pm on August 31, 2018: member

Could you also upload both of the tar.gz results?

ken2812221 commented at 1:25 pm on August 31, 2018: contributor

https://transfer.sh/GZOYX/native_nsis-1.tar.gz https://transfer.sh/14L552/native_nsis-2.tar.gz

ken2812221 renamed this:
~~wip, depends: Add native_nsis to support unicode~~
[WIP] depends: Add native_nsis to support unicode
on Sep 11, 2018

ken2812221 commented at 6:53 am on September 16, 2018: contributor

Close it for now. I can’t solve the gitian build deterministic issue.

ken2812221 closed this on Sep 16, 2018

ken2812221 deleted the branch on Sep 16, 2018

fanquake added the label Up for grabs on Sep 16, 2018

MarcoFalke commented at 1:53 pm on September 16, 2018: member

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? At least that should be the goal when we want to support deterministic builds on any host by default.

If you reopen, I can run DrahtBot on this twice to see what the effective difference is for Gitian builds.

ken2812221 restored the branch on Sep 16, 2018

ken2812221 commented at 4:20 pm on September 16, 2018: contributor

Why is it required that the depends cache is deterministic?

It has to compile installer and decompressor that runs on Windows.

If you reopen, I can run DrahtBot on this twice to see what the effective difference is for Gitian builds.

Thanks, but you have to clear nsis depends cache manually, otherwise it’s deterministic.

ken2812221 reopened this on Sep 16, 2018

DrahtBot added the label Needs rebase on Oct 26, 2018

DrahtBot commented at 1:19 pm on October 26, 2018: member

ken2812221 closed this on Oct 26, 2018

ken2812221 deleted the branch on Oct 26, 2018

laanwj removed the label Needs rebase on Oct 24, 2019

MarcoFalke commented at 2:05 pm on February 6, 2020: member

Does this upstream patch fix this? https://sourceforge.net/p/nsis/bugs/1230/

fanquake removed the label Up for grabs on Mar 2, 2021

fanquake commented at 8:55 am on March 2, 2021: member

Removing “Up for grabs”, as this should be taken care of by #21333.

DrahtBot locked this on Aug 16, 2022

[WIP] depends: Add native_nsis to support unicode #13827