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

Sync to upstream #8

Merged
merged 7 commits into from
Aug 30, 2024
Merged

Conversation

fanquake
Copy link
Member

Pulls the few changes from upstream (last commit ~ 2 years ago).
Seems reasonable to do before making any other changes. i.e #7, or if we are going to make further changes to the CMake build system etc.

pwnall and others added 7 commits August 30, 2021 19:11
* Fix Windows CI build.

The Windows SDK versions present on our CI options (GitHub Actions
hosted runners, AppVeyor) have headers that cause compilation warnings
when included with MSVC operating in modern C modes (C11+).

This PR disables the compilation warning caused by the Windows SDK
headers, so we can get Windows feedback from CI. The warnings can be
re-enabled when the Windows SDK used by our CI is upgraded to a version
that doesn't trigger warnings.

* Switch CI to GitHub Actions.
The Windows SDK versions present on our CI options (GitHub Actions
hosted runners, AppVeyor) have headers that cause compilation warnings
when included with MSVC operating in modern C modes (C11+).

This PR disables the compilation warning caused by the Windows SDK
headers, so we can get Windows feedback from CI. The warnings can be
re-enabled when the Windows SDK used by our CI is upgraded to a version
that doesn't trigger warnings.
Copy link
Member

@willcl-ark willcl-ark left a comment

Choose a reason for hiding this comment

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

The CI + CMakeLists.txt as they stand here are not worth adding: https://github.com/willcl-ark/crc32c-subtree/actions/runs/10605521278/job/29394471127

But tests and benches can be relatively easily hacked(1) out(2) to get an array of simple build tests passing again: https://github.com/willcl-ark/crc32c-subtree/actions/runs/10605602819

(I assume this is preferable to merging 3rd party deps).

The rest is trivial enough.

@@ -1,7 +1,6 @@
# CRC32C

[![Build Status](https://travis-ci.org/google/crc32c.svg?branch=master)](https://travis-ci.org/google/crc32c)
[![Build Status](https://ci.appveyor.com/api/projects/status/moiq7331pett4xuj/branch/master?svg=true)](https://ci.appveyor.com/project/pwnall/crc32c)
[![Build Status](https://github.com/google/crc32c/actions/workflows/build.yml/badge.svg)](https://github.com/google/crc32c/actions/workflows/build.yml)
Copy link
Member

Choose a reason for hiding this comment

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

This isn't correct for us, but can be fixed later

@fanquake
Copy link
Member Author

The point is just to pull the upstream changes. Any other (relevant) changes could be done in a followup.

@willcl-ark
Copy link
Member

Yes I agree, in this case it looks fine to me. 👍🏼

Ripping out the third party stuff from the CMakLists and CI is easy enough to do later.

@fanquake fanquake merged commit efb8ea0 into bitcoin-core:bitcoin-fork Aug 30, 2024
@fanquake fanquake deleted the sync_to_upstream branch August 30, 2024 10:11
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.

5 participants