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

Windows build C PoW doesn't work #1919

Closed
PeterSurda opened this issue Jan 18, 2022 · 17 comments · Fixed by #1937
Closed

Windows build C PoW doesn't work #1919

PeterSurda opened this issue Jan 18, 2022 · 17 comments · Fixed by #1937
Assignees
Labels
bug Something isn't working as it's supposed to build Related to building or build system duplicate

Comments

@PeterSurda
Copy link
Member

I got a report that the windows executables report C Pow missing. I tried it in Windows Server 2012 and 2016 and couldn't reproduce it, but the person reporting claims on Windows 7 and Windows 10 they get an error message.

@PeterSurda PeterSurda added the bug Something isn't working as it's supposed to label Jan 18, 2022
@g1itch
Copy link
Collaborator

g1itch commented Jan 18, 2022

Cool! Let them try the exe from my release. Because I also could not test it properly.

@BeholdersEye
Copy link

BeholdersEye commented Jan 21, 2022

@g1itch I am the person who reported it and can confirm that the release you linked:

  1. Does not display the C pow missing message.
  2. Creates identities / sends messages fast enough that I am comfortable saying it does have the working proof of work module (snapshots that display the message create identities and send messages very slowly).

In other words, working as expected.

@g1itch
Copy link
Collaborator

g1itch commented Jan 22, 2022

@g1itch I am the person who reported it and can confirm that the release you linked:

1. Does not display the C pow missing message.

...

Thanks, I read the reddit.

The difference in my release is Microsoft visual C compiler for python27 used to compile the ext: https://github.com/g1itch/PyBitmessage/tree/windows-binary
Also I cannot confirm that the issue appeared in November 2021, I started seeing it in 2019.

@g1itch g1itch added build Related to building or build system duplicate labels Jan 22, 2022
@g1itch
Copy link
Collaborator

g1itch commented Jan 22, 2022

The first mention seems to be #1622

@PeterSurda
Copy link
Member Author

I suspect in this case it's also related to a CPU. Maybe some compilation flags can work around it.

@BeholdersEye
Copy link

@G1litch:

Also I cannot confirm that the issue appeared in November 2021, I started seeing it in 2019.

I don't meant to say that Nov. 2021 was the first appearance, only that in my testing of the snapshots @

https://download.bitmessage.org/snapshots/

"20211126" does not display the problem and every later snapshot does.

@PeterSurda:

I tested the 20220125 snapshot and it still shows the C PoW error message.

@BeholdersEye
Copy link

@PeterSurda: No change on today's snapshot. (20220210)

@BeholdersEye
Copy link

@PeterSurda: No change on today's snapshot. (20220216)

Perhaps consulting the changelogs around the time of the 20211126 build might help diagnose the problem.

@PeterSurda
Copy link
Member Author

@BeholdersEye There isn't anyone working on this issue for the time being. That being said, my educated guess is that this is caused by the dll compiler options at https://github.com/Bitmessage/PyBitmessage/blob/v0.6/buildscripts/winbuild.sh#L151 and https://github.com/Bitmessage/PyBitmessage/blob/v0.6/buildscripts/winbuild.sh#L157 . The build is done on a Zen 2 CPU so it probably tries to use instructions that Zen 2 understands but your CPU doesn't. Someone with adequate expertise can probably write a fix very quickly.

@BeholdersEye
Copy link

BeholdersEye commented Feb 16, 2022

I agree that does appear to be the problem area. Specifically "march=native". Quoth the documentation:

https://gcc.gnu.org/onlinedocs/gcc/x86-Options.html

Using -march=native enables all instruction subsets supported by the local machine (hence the result might not run on different machines).

In the case of building for 64-bit targets, the march=x86-64 option seems more appropriate for release builds that are expected to be run on machines other than those on which the binaries were built.

‘x86-64’

A generic CPU with 64-bit extensions.

Or, in the case of building for 32-bit targets, the march=i686 option.

‘i686’

When used with -march, the Pentium Pro instruction set is used, so the code runs on all i686 family chips. When used with -mtune, it has the same meaning as ‘generic’.

Can this change be implemented, please? As you say, implementing the fix is not especially difficult but I suspect reproducing the build environment for bitmessage on windows might be more challenging.

@g1itch
Copy link
Collaborator

g1itch commented Feb 16, 2022

Hmm, I agree. march=native is definitely a mistake.

@g1itch
Copy link
Collaborator

g1itch commented Feb 21, 2022

I've built another release with march=native removed.

@BeholdersEye
Copy link

@g1itch: that release's proof of work is functional, though the release in your earlier reply also had functional proof-of-work.

I am using bitmessage in lieu of email on a website and would like to be able to recommend / point to the "official" release of bitmessage (which I take this repository to represent) to the website's users without saying "Make sure to download the snapshot from November 26, 2021 unless you have such-and-such CPU."

It is possible that many users won't even know their CPU's manufacturer so expecting them to pick the right version of bitmessage based on their processor's microarchitecture is optimistic.

@PeterSurda in case mentioning is necessary to notify someone on github.

@PeterSurda
Copy link
Member Author

@BeholdersEye if you want, you can make a pull request, then I can have the windows binary built and you can test it

@BeholdersEye
Copy link

@PeterSurda

Although I will if it is absolutely necessary, if at all possible I would rather avoid taking the trouble to fork this repository and make the changes just to correct two lines of code.

Is opening the issue and proposing the solution not sufficient to get this addressed?

@BeholdersEye
Copy link

BeholdersEye commented Feb 22, 2022

Went ahead and made the pull request. It is possible the change might not solve the problem which would mean I may need to submit further changes in the future.

@g1itch
Copy link
Collaborator

g1itch commented Feb 22, 2022

I am using bitmessage in lieu of email on a website and would like to be able to recommend / point to the "official" release of bitmessage (which I take this repository to represent) to the website's users without saying "Make sure to download the snapshot from November 26, 2021 unless you have such-and-such CPU."

I think, that changing the march argument is more obvious fix, so it has more chances to merge into v0.6 soon. And then any snapshot on https://download.bitmessage.org will be compatible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working as it's supposed to build Related to building or build system duplicate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants