Skip to content

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

Merged
fanquake merged 1 commit into
bitcoin:masterfrom
sipsorcery:msvc-build-readme-update
Dec 11, 2023
Merged

Add a note to msvc readme re building Qt for Bitcoin Core.#29048
fanquake merged 1 commit into
bitcoin:masterfrom
sipsorcery:msvc-build-readme-update

Conversation

@sipsorcery

@sipsorcery sipsorcery commented Dec 10, 2023

Copy link
Copy Markdown
Contributor

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

DrahtBot commented Dec 10, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

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.

hebasto
hebasto approved these changes Dec 10, 2023
View reviewed changes

@hebasto hebasto left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ACK a9b9fb2eeedbd9094d74f8f429622ba7d600dda3, 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

hebasto commented Dec 10, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

@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

sipsorcery commented Dec 10, 2023

Copy link
Copy Markdown
Contributor 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

hebasto commented Dec 10, 2023

Copy link
Copy Markdown
Member

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

hebasto commented Dec 10, 2023

Copy link
Copy Markdown
Member

@sipsorcery

We make such changes in a single commit usually :)

Mind squashing your commits?

@sipsorcery
sipsorcery force-pushed the msvc-build-readme-update branch from aaf338e to d08e820 Compare December 10, 2023 15:46
hebasto
hebasto approved these changes Dec 10, 2023
View reviewed changes

@hebasto hebasto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ACK d08e820.

sedited
sedited approved these changes Dec 10, 2023
View reviewed changes

@sedited sedited left a comment

Copy link
Copy Markdown
Contributor

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
@bitcoin bitcoin locked and limited conversation to collaborators Dec 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@sedited sedited sedited approved these changes
@hebasto hebasto hebasto approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@sipsorcery @DrahtBot @hebasto @sedited @fanquake