Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gitian: Various improvements for Windows descriptor #17029

Merged
merged 5 commits into from Oct 9, 2019

Conversation

@dongcarl
Copy link
Contributor

commented Oct 2, 2019

It would seem that our gitian-win.yml has not been keeping up with gitian-linux.yml, this PR:

  1. Minimizes the diff size between gitian-{win,linux}.yml
  2. Eliminates the rename dependency
Debug splitting was first introduced in 7e7eb27, then gitian-linux.yml
changed to using split-debug.sh in 9d25362. Here we change
gitian-win.yml to use split-debug.sh as well.
@jonasschnelli

This comment has been minimized.

Copy link
Member

commented Oct 3, 2019

@fanquake

This comment has been minimized.

Copy link
Member

commented Oct 4, 2019

Concept ACK

1 similar comment
@hebasto

This comment has been minimized.

Copy link
Member

commented Oct 6, 2019

Concept ACK

dongcarl added 4 commits Oct 2, 2019
Linux:
The README was originally added in 8550f1f, but included the README
under the docs directory, which has a bunch of internal links that won't
make sense in a release tarball. In this patch, we include the root
level README instead, which makes more sense.

Windows:
.md files are inconvenient to open on windows and the line endings
differ, so we use README_windows.txt instead.
@dongcarl dongcarl force-pushed the dongcarl:2019-10-gitian-win-improvements branch from ee69d93 to 9d1f971 Oct 8, 2019
@dongcarl

This comment has been minimized.

Copy link
Contributor Author

commented Oct 8, 2019

Addressed all comments.

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Oct 8, 2019

See also #17077 (slightly related)

@hebasto

This comment has been minimized.

Copy link
Member

commented Oct 8, 2019

See also #17077 (slightly related)

Yes. The question (#17077) arose while reviewing this PR ;)

@bitcoin bitcoin deleted a comment from DrahtBot Oct 8, 2019
@dongcarl

This comment has been minimized.

Copy link
Contributor Author

commented Oct 8, 2019

I know I'm kinda asking for it because of naming of the PR, but could we resolve #17077 in another PR? 😬

@DrahtBot

This comment has been minimized.

Copy link
Contributor

commented Oct 9, 2019

Gitian builds for commit e173d58 (master):

Gitian builds for commit 531a3dc (master and this pull):

@laanwj

This comment has been minimized.

Copy link
Member

commented Oct 9, 2019

I think that can very well be addressed separately.
It's not good to have a PR that keeps increasing in scope.

ACK 9d1f971

laanwj added a commit that referenced this pull request Oct 9, 2019
9d1f971 gitian: Put things in the right place to begin with (Carl Dong)
71949a9 gitian: Eliminate rename dependency (Carl Dong)
999a9a5 gitian: Smaller diff with gitian-linux.yml (Carl Dong)
c4a3c25 gitian: Fix README inclusion in archives (Carl Dong)
93cb974 gitian: Use split-debug.sh for Win builds (Carl Dong)

Pull request description:

  It would seem that our `gitian-win.yml` has not been keeping up with `gitian-linux.yml`, this PR:

  1. Minimizes the diff size between `gitian-{win,linux}.yml`
  2. Eliminates the `rename` dependency

ACKs for top commit:
  laanwj:
    ACK 9d1f971

Tree-SHA512: 84ed47c685e12d0064c02811907ae3d0fd3c47db8773d497dcc38f0defbfb3040fd82899fb026cf355f229b906d05a1c8038a95642bb90d044afbc2e0b239af2
@laanwj laanwj merged commit 9d1f971 into bitcoin:master Oct 9, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
(
SETUP_EXE="$(basename "$(echo ./*-setup.exe)")"
cp -f "$SETUP_EXE" "${OUTDIR}/${SETUP_EXE/%-setup.exe/-setup-unsigned.exe}"
)

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Oct 9, 2019

Member

Any thoughts on removing this?

diff --git a/contrib/gitian-descriptors/gitian-win.yml b/contrib/gitian-descriptors/gitian-win.yml
index d5f2c1ad35..2d613fca9d 100644
--- a/contrib/gitian-descriptors/gitian-win.yml
+++ b/contrib/gitian-descriptors/gitian-win.yml
@@ -153,10 +153,6 @@ script: |
     make ${MAKEOPTS} -C src check-security
     make deploy
     make install DESTDIR=${INSTALLPATH}
-    (
-      SETUP_EXE="$(basename "$(echo ./*-setup.exe)")"
-      cp -f "$SETUP_EXE" "${OUTDIR}/${SETUP_EXE/%-setup.exe/-setup-unsigned.exe}"
-    )
     cd installed
     mv ${DISTNAME}/bin/*.dll ${DISTNAME}/lib/
     find . -name "lib*.la" -delete
diff --git a/share/setup.nsi.in b/share/setup.nsi.in
index e9aa1f2b73..649483c732 100644
--- a/share/setup.nsi.in
+++ b/share/setup.nsi.in
@@ -48,7 +48,7 @@ Var StartMenuGroup
 !insertmacro MUI_LANGUAGE English
 
 # Installer attributes
-OutFile @abs_top_srcdir@/@PACKAGE_TARNAME@-@PACKAGE_VERSION@-win@WINDOWS_BITS@-setup.exe
+OutFile @abs_top_srcdir@/@PACKAGE_TARNAME@-@PACKAGE_VERSION@-win@WINDOWS_BITS@-setup-unsigned.exe
 !if "@WINDOWS_BITS@" == "64"
 InstallDir $PROGRAMFILES64\Bitcoin
 !else
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.