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.

Sign up for GitHub

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

Jump to bottom

build: 🚀 Remove Autotools and MSVC build systems #166

Closed
wants to merge 2 commits into from
Closed

build: 🚀 Remove Autotools and MSVC build systems #166

wants to merge 2 commits into from

Conversation

hebasto
Copy link
Owner

@hebasto hebasto commented Apr 23, 2024

The ultimate goal :)

@hebasto hebasto changed the title build: Remove Autotools-based build system build: 🚀 Remove Autotools-based build system Apr 24, 2024
vasild
vasild approved these changes Apr 24, 2024
View reviewed changes
Copy link

@vasild vasild left a comment

Choose a reason for hiding this comment

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

ACK 51706e0

I guess if something is forgotten it can be removed later and it looks like nothing excessive is removed either (but doc/Doxygen file generation?)

.gitignore
.DS_Store
build
/build*
!/build-aux
Copy link

Choose a reason for hiding this comment

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

Why unignore /build-aux?

Copy link
Owner Author

Choose a reason for hiding this comment

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

To ensure that Autotools legacy won't sneak into CMake realm.

doc/.gitignore Show resolved Hide resolved
@hebasto hebasto mentioned this pull request Apr 27, 2024
Merged
@hebasto hebasto force-pushed the cmake-kill-autotools branch 2 times, most recently from 3a58eea to 9186936 Compare April 30, 2024 19:40
hebasto added a commit that referenced this pull request May 1, 2024
@hebasto
Merge #174: cmake, doc: Update depends/README.md
ce7e236 
fa4f450 cmake, doc: Update `depends/README.md` (Hennadii Stepanov)

Pull request description:

  Split from #166.

ACKs for top commit:
  m3dwards:
    ACK fa4f450

Tree-SHA512: fd1aff3ee2ac3bc9cf5ea9ceeb5cf4490325935e404e4f2b6cbf44ea9186f062bbc169f9772b812f2bb1a03728699c7f361a7a07d7c2d89f73246b54993a639c
@hebasto
Copy link
Owner Author

hebasto commented May 1, 2024

Unit tests create an artifact in the source tree:

share/rpcauth/__pycache__/rpcauth.cpython-312.pyc

@hebasto
Copy link
Owner Author

hebasto commented May 3, 2024

Unit tests create an artifact in the source tree:

share/rpcauth/__pycache__/rpcauth.cpython-312.pyc

Fixed.

vasild
vasild approved these changes May 14, 2024
View reviewed changes
Copy link

@vasild vasild left a comment

Choose a reason for hiding this comment

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

ACK 7752690

@hebasto
Copy link
Owner Author

hebasto commented May 17, 2024

Rebased. More stuff have been removed from the src/univalue .

pablomartin4btc
pablomartin4btc reviewed May 23, 2024
View reviewed changes
Copy link

@pablomartin4btc pablomartin4btc left a comment

Choose a reason for hiding this comment

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

ACK 44296a2

vasild
vasild approved these changes May 31, 2024
View reviewed changes
Copy link

@vasild vasild left a comment

Choose a reason for hiding this comment

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

ACK 44296a2

@hebasto hebasto force-pushed the cmake-kill-autotools branch 2 times, most recently from dce1045 to 045f267 Compare June 8, 2024 23:25
@vasild
Copy link

vasild commented Jun 20, 2024

ACK 045f267

@hebasto hebasto changed the title build: 🚀 Remove Autotools-based build system build: 🚀 Remove Autotools and MSVC build systems Jul 26, 2024
@hebasto hebasto mentioned this pull request Aug 5, 2024
Merged
hebasto added a commit that referenced this pull request Aug 6, 2024
@hebasto
Merge #302: cmake: Ignore build subdirectories within the source dire…
d847137 
…ctory

57cdcdf cmake: Ignore build subdirectories within source directory (Hennadii Stepanov)

Pull request description:

  It was planned to clean up the `.gitignore` file during the [removal](#166) of the Autotools and MSVC legacy build systems. However, reviewers requested it [earlier](bitcoin#30454 (comment)).

  Therefore, [delivering](bitcoin#30454 (comment)) it now :)

ACKs for top commit:
  paplorinc:
    ACK 57cdcdf

Tree-SHA512: 7279debfca02b4dad13f5ec68190f09c16a34d9079d5859163f28b55193ef4e32a9bb925f104b9d83a01782146482ab9fd137c2af987b4d38c00e61c8a018026
vasild
vasild approved these changes Aug 14, 2024
View reviewed changes
Copy link

@vasild vasild left a comment

Choose a reason for hiding this comment

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

ACK 3e6cf60

Looks good, also the two new commits since my last review.

@vasild
Copy link

vasild commented Aug 16, 2024

Why restore src/minisketch/.gitignore and why drop commit 3e6cf60 test: Do not write Python bytecode to source directory?

@hebasto
Copy link
Owner Author

hebasto commented Aug 16, 2024

Why restore src/minisketch/.gitignore..?

The src/minisketch is a subtree and shouldn't be modified in this PR.

and why drop commit 3e6cf60 test: Do not write Python bytecode to source directory?

It is still not clear which way is preferred. For more details, please refer to bitcoin#30533.

vasild
vasild approved these changes Aug 16, 2024
View reviewed changes
Copy link

@vasild vasild left a comment

Choose a reason for hiding this comment

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

ACK d3f1c8a

@hebasto
Copy link
Owner Author

hebasto commented Aug 17, 2024

Rebased.

@hebasto hebasto mentioned this pull request Aug 18, 2024
Merged
vasild
vasild approved these changes Aug 19, 2024
View reviewed changes
Copy link

@vasild vasild left a comment

Choose a reason for hiding this comment

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

ACK ad12ccc

@hebasto hebasto mentioned this pull request Aug 19, 2024
Closed
pablomartin4btc
pablomartin4btc approved these changes Aug 26, 2024
View reviewed changes
Copy link

@pablomartin4btc pablomartin4btc left a comment

Choose a reason for hiding this comment

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

ACK ad12ccc

Tested it building depends with make and cmake building using the toolchain.

@maflcko
Copy link

maflcko commented Aug 28, 2024

Maybe the two commits can be split up into two pull requests? MSVC removal should be doable now, while autotools may or may not be good to keep a few more days, given my comment on the merged cmake pull.

@maflcko
Copy link

maflcko commented Aug 28, 2024

Looks like this is now bitcoin#30731 and bitcoin#30664, so reviewers of this change may want to go there.

@hebasto
Copy link
Owner Author

hebasto commented Aug 28, 2024

Looks like this is now bitcoin#30731 and bitcoin#30664, so reviewers of this change may want to go there.

Closing this PR.

@hebasto hebasto closed this Aug 28, 2024
This was referenced Aug 28, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vasild vasild vasild approved these changes

@pablomartin4btc pablomartin4btc pablomartin4btc 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.
4 participants
@hebasto @vasild @maflcko @pablomartin4btc