Hardcode string in miniupnpcstring.h and remove wine from deps-win32.yml #3042

wtogami commented at 8:04 AM on September 30, 2013: contributor

Credit: sipa, cfields, Luke-Jr and michagogo contributed to this fix.

Removing wine as a win32 build dep is beneficial to fixing Gitian LXC, which would enable many more people to join in the gitian.sigs process by being able to build with a linux VM instead of a virt capable host.

laanwj commented at 8:09 AM on September 30, 2013: member

ACK, good thing to get rid of wine here

in contrib/gitian-descriptors/deps-win32.yml:None in 880ffe6f0d outdated

  56 | +   wingenminiupnpcstrings.o:	wingenminiupnpcstrings.c
  57 | +   
  58 | +  -miniupnpcstrings.h: miniupnpcstrings.h.in wingenminiupnpcstrings
  59 | +  -	wingenminiupnpcstrings $< $@
  60 | +  +miniupnpcstrings.h: miniupnpcstrings.h.in 
  61 | +  +	sed -e 's|OS/version|MSWindows/5.1.2600|' -e 's|MINIUPNPC_VERSION_STRING "version"|MINIUPNPC_VERSION_STRING "1.8"|' $< > $@

sipa commented at 9:29 AM on September 30, 2013:

Currently the code is still at 1.6, so don't hardcode 1.8. A comment to update this when the source is updated would also be nice.

sipa commented at 9:31 AM on September 30, 2013: member

Also, why not wait with bumping the version until we actually do update the sources (to 1.8, or whatever is decided). It shouldn't matter for now.

wtogami commented at 9:35 AM on September 30, 2013: contributor

I bumped the .zip revision because the payload actually was different.

Good catch, I forgot to s/1.8/1.6/ with this PR that doesn't actually upgrade miniupnpc yet. Should we do the same thing as the previous PR, one commit for the wine removal, and another to upgrade to 1.8?

wtogami commented at 10:13 AM on September 30, 2013: contributor

Fixed the patch to have 1.6 instead of 1.8. I don't have time to test 1.8 right now, so let's limit this PR only to wine removal for now.

theuni commented at 4:08 PM on September 30, 2013: member

Since there are a few of us working on build/gitian related changes, I'd recommend not merging these right away, just let them stack up. Maybe give them some kind of tag on github.

It'll only serve to confuse to bump the version numbers a half-dozen times before settling down.

in contrib/gitian-descriptors/deps-win32.yml:None in c34cd8865c outdated

  45 | @@ -47,10 +46,24 @@ script: |
  46 |    #
  47 |    tar xzf miniupnpc-1.6.tar.gz
  48 |    cd miniupnpc-1.6
  49 | -  sed 's/dllwrap -k --driver-name gcc/$(DLLWRAP) -k --driver-name $(CC)/' -i Makefile.mingw
  50 | -  sed 's|wingenminiupnpcstrings $< $@|./wingenminiupnpcstrings $< $@|' -i Makefile.mingw
  51 | +  # Note: Please be sure to update the MINIUPNPC_VERSION_STRING to match the version when this library is updated.

theuni commented at 4:15 PM on September 30, 2013:

The need for manual intervention here could be avoided by using cat VERSION.

That's what the linux build does (see updateminiupnpcstrings.sh). Then there would be no accidents when bumping.

laanwj commented at 6:01 AM on October 1, 2013:

Good catch @theuni, the less manual intervention needed the less ways to screw up.

Hardcode string in miniupnpcstring.h and remove wine requirement from deps-win32.yml

Credit: sipa, cfields, Luke-Jr and michagogo contributed to this fix.

Removing wine as a win32 build dep is beneficial to fixing Gitian LXC,
which would enable many more people to join in the gitian.sigs process
by being able to build with a linux VM instead of a virt capable host.

738b8598ae

wtogami commented at 8:24 AM on October 1, 2013: contributor

VERSION file now supplies the string to the patch file. This is now ready for commit. This will be the last gitian change in the next few days to stop the churn and allow cfields to do the cleanups he wants.

laanwj referenced this in commit 95a766abc5 on Oct 1, 2013

laanwj merged this on Oct 1, 2013

laanwj closed this on Oct 1, 2013

DrahtBot locked this on Sep 8, 2021