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

Update winbuild.sh #1933

Closed
wants to merge 4 commits into from
Closed

Update winbuild.sh #1933

wants to merge 4 commits into from

Conversation

BeholdersEye
Copy link

changes proof of work gcc machine architecture setting from "native" to "x86-64" and "i686" as appropriate to allow the resultant binary to run on CPU's other than the on which the binary was compiled.

#1919

@@ -134,27 +134,27 @@ function build_dll(){
cd src/bitmsghash || exit 1
if [ "${MACHINE_TYPE}" == 'x86_64' ]; then
echo "Create dll"
x86_64-w64-mingw32-g++ -D_WIN32 -Wall -O3 -march=native \
x86_64-w64-mingw32-g++ -D_WIN32 -Wall -O3 -march=x86-64 \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just remove?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that not what the minus signs indicate?

I sure didn't intend to leave "march=native" so hopefully github is not doing that.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, aren't those values the defaults, that will be applied if you just remove the -march=native? Though maybe it's better to set that explicitly.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It turns out that there is no default value for the march option.

There is no default in general, because it's platform-specific.

https://gcc-help.gcc.gnu.narkive.com/Z259fPcD/default-value-for-march-option

Though you can find what your platform will use as default (if it is not specified) with the command:

gcc -Q --help=target

https://stackoverflow.com/questions/27786932/what-is-the-default-for-gcc-march-option

@g1itch
Copy link
Collaborator

g1itch commented Feb 22, 2022

You might want to add something like "(closes: #1919)" into the commit message.

@g1itch g1itch added build Related to building or build system performance labels Feb 22, 2022
@BeholdersEye
Copy link
Author

You might want to add something like "(closes: #1919)" into the commit message.

I am not certain this closes #1919 yet but if it does I will keep that in mind.

@g1itch
Copy link
Collaborator

g1itch commented Feb 22, 2022

You might want to add something like "(closes: #1919)" into the commit message.

I am not certain this closes #1919 yet but if it does I will keep that in mind.

I thought you could just download a build, because buildbot/PyBitmessage-wine32 is finished successfully. But https://download.bitmessage.org/builds is not available today );

@BeholdersEye
Copy link
Author

You might want to add something like "(closes: #1919)" into the commit message.

I am not certain this closes #1919 yet but if it does I will keep that in mind.

I thought you could just download a build, because buildbot/PyBitmessage-wine32 is finished successfully. But https://download.bitmessage.org/builds is not available today );

https://download.bitmessage.org/snapshots/ is available and there is a new build for today there but I am uncertain whether that build incorporates this pull request as it has not yet been merged.

@g1itch
Copy link
Collaborator

g1itch commented Feb 22, 2022

https://download.bitmessage.org/snapshots/ is available and there is a new build for today there but I am uncertain whether that build incorporates this pull request as it has not yet been merged.

snapshots are for merges into v0.6, so you cannot check the snapshot before the merge

@PeterSurda
Copy link
Member

I can tell buildbot to build a debug binary and it will then be available for upload

@PeterSurda
Copy link
Member

PeterSurda commented Feb 23, 2022

Binary from this PR: https://storage.bitmessage.org/binaries/Bitmessage_x86_7652.exe
It's a debug binary so it will open a console window and print a lot of log data, maybe it will even give helpful information in case it still doesn't work correctly.

@BeholdersEye
Copy link
Author

Binary from this PR: https://storage.bitmessage.org/binaries/Bitmessage_x86_7652.exe It's a debug binary so it will open a console window and print a lot of log data, maybe it will even give helpful information in case it still doesn't work correctly.

Wonderful. Tested on the machines that displayed the C PoW missing message in the status and that were (effectively) unable to send messages due to proof of work taking forever. They no longer display the C PoW missing message and messages are sent as expected.

Pending merge, this closes #1919.

@PeterSurda
Copy link
Member

@BeholdersEye What's left to do is to sign the commit and rebase the PR. If you're having problems with that, I can assign someone to do that instead.

@BeholdersEye
Copy link
Author

@BeholdersEye What's left to do is to sign the commit and rebase the PR. If you're having problems with that, I can assign someone to do that instead.

@PeterSurda By all means do so.

@BeholdersEye
Copy link
Author

BeholdersEye commented Feb 24, 2022

I added a gpg signature to my account but if you require email verification to fix bugs then I will let someone else do it. Feels like having a state funeral for a mosquito.

@PeterSurda
Copy link
Member

The signatures are mainly used to maintain the integrity of the repository rather than verifying the identity of contributors. If I make an exception then some automated scripts will stop working (i.e. it will look like someone is trying to change the repository without authorisation). I understand it's is a bit of a hassle for new contributors, I plan on adding it to the introductory videos so that it's easier.

@g1itch
Copy link
Collaborator

g1itch commented Feb 24, 2022 via email

@BeholdersEye
Copy link
Author

sign the commit and rebase the PR. If you're having problems with that, I can assign someone to do that instead.

@PeterSurda please do so.

@BeholdersEye
Copy link
Author

BeholdersEye commented Feb 24, 2022

@g1itch

You might rebase and sign by your own.

I get the impression that the problem is that my gpg signature is not associated with a verified email address.

https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification

https://docs.github.com/en/authentication/managing-commit-signature-verification/associating-an-email-with-your-gpg-key

Too bad it is not possible to verify with a bitmessage address.

@BeholdersEye
Copy link
Author

BeholdersEye commented Feb 25, 2022

@PeterSurda

I would rather not share any more information on github.

Let me know if you need any more help from me to get this implemented in a release build.

@PeterSurda
Copy link
Member

@BeholdersEye no need for you to do anything, I assigned a developer to the task, it may just take a couple of days

@PeterSurda
Copy link
Member

Closed with #1937

@PeterSurda PeterSurda closed this Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Related to building or build system performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants