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

Add a note to msvc readme re building Qt for Bitcoin Core. #29048

Merged
merged 1 commit into from
Dec 11, 2023

Conversation

sipsorcery
Copy link
Member

Updated the msvc readme with a note about avoiding path too long errors when building Qt with Bitcoin Core.

Would have saved me half an hour if I'd remembered this from the last time I did the build.

@DrahtBot
Copy link
Contributor

DrahtBot commented Dec 10, 2023

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK hebasto, TheCharlatan

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK a9b9fb2, the tip is correct and useful.

However, it won't be needed after migration to CMake as vcpkg's Qt package will be used then.

@hebasto
Copy link
Member

hebasto commented Dec 10, 2023

@sipsorcery

There is a linter error in CI:

File "build_msvc/README.md" has permission 755 (executable) and is thus expected to contain a shebang '#!'. Add shebang or do "chmod 644 build_msvc/README.md" to make it non-executable.

@sipsorcery
Copy link
Member Author

@sipsorcery

There is a linter error in CI:

Even after I manually checked for whitespace violations on that single line addition :(.

Although it does seem like the linter culprit may be elsewhere? Or, am I missing something?

src/test/fuzz/package_eval.cpp:214: non-existant ==> non-existent
src/test/span_tests.cpp:56: memeber ==> member
^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/spelling.ignore-words.txt
File "build_msvc/README.md" has permission 755 (executable) and is thus expected to contain a shebang '#!'. Add shebang or do "chmod 644 build_msvc/README.md" to make it non-executable.
ERROR: There were 1 failed tests in the lint-files.py lint test. Please resolve the above errors.
^---- failure generated from lint-files.py

@hebasto
Copy link
Member

hebasto commented Dec 10, 2023

Or, am I missing something?

Just rewording the linter:

The problem is:

$ stat build_msvc/README.md 
  File: build_msvc/README.md
  Size: 5650      	Blocks: 16         IO Block: 4096   regular file
Device: 10303h/66307d	Inode: 14949723    Links: 1
Access: (0755/-rwxr-xr-x)  Uid: ( 1001/ hebasto)   Gid: ( 1001/ hebasto)
Access: 2023-12-10 15:34:07.780145234 +0000
Modify: 2023-12-10 15:34:06.520175365 +0000
Change: 2023-12-10 15:34:06.520175365 +0000
 Birth: 2023-12-10 15:34:06.520175365 +0000

The fix is:

$ chmod 644 build_msvc/README.md

(the POSIX environment is assumed)

@hebasto
Copy link
Member

hebasto commented Dec 10, 2023

@sipsorcery

We make such changes in a single commit usually :)

Mind squashing your commits?

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK d08e820.

Copy link
Contributor

@TheCharlatan TheCharlatan left a comment

Choose a reason for hiding this comment

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

ACK d08e820

@fanquake fanquake merged commit 84bbee7 into bitcoin:master Dec 11, 2023
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants