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

Fix Windows CI build #54

Merged
merged 1 commit into from
Sep 1, 2021
Merged

Fix Windows CI build #54

merged 1 commit into from
Sep 1, 2021

Conversation

pwnall
Copy link
Member

@pwnall pwnall commented Aug 31, 2021

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.

@google-cla google-cla bot added the cla: yes label Aug 31, 2021
@pwnall pwnall requested a review from cmumford August 31, 2021 16:26
@pwnall
Copy link
Member Author

pwnall commented Aug 31, 2021

@cmumford PTAL?

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.
@pwnall pwnall changed the title Fix AppVeyor build Fix Windows CI build Sep 1, 2021
@pwnall
Copy link
Member Author

pwnall commented Sep 1, 2021

@cmumford I updated this PR to clarify that we still need it for Github Actions. PTAL?

Copy link
Contributor

@cmumford cmumford left a comment

Choose a reason for hiding this comment

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

LGTM

@cmumford cmumford merged commit b9d6e82 into main Sep 1, 2021
@pwnall
Copy link
Member Author

pwnall commented Sep 1, 2021

Thank you very much for the quick review!

@pwnall pwnall deleted the fix-appveyor branch September 1, 2021 16:24
swtrse added a commit to muecoin/crc32c-subtree that referenced this pull request Sep 24, 2021
commit b9d6e82
Author: Victor Costan <costan@google.com>
Date:   Wed Sep 1 09:19:47 2021 -0700

    Fix Windows CI build. (google#54)

    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.

commit bbbb93a
Author: Victor Costan <costan@google.com>
Date:   Wed Sep 1 09:17:07 2021 -0700

    Switch CI to GitHub Actions (google#55)

    * 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.

commit d46cd17
Author: Victor Costan <pwnall@chromium.org>
Date:   Mon Aug 30 19:11:12 2021 -0700

    Add clangd cache directory to .gitignore.

commit fa5ade4
Author: Victor Costan <costan@google.com>
Date:   Tue Jun 22 14:30:59 2021 -0700

    Fix compilation warnings on ARM64 with old GCC versions. (google#52)

    Tested on Chrome OS M93 Linux (Debian 10.10) on kukui.

commit db08d22
Author: Chris Mumford <cmumford@cmumford.com>
Date:   Sun Jun 20 08:18:01 2021 -0700

    Updated Travis-CI configuration. (google#51)

    clang/llvm: 10 → 12
    gcc: 10 → 11

commit e31619a
Author: Francois Marier <francois@fmarier.org>
Date:   Wed Jun 2 12:18:58 2021 -0700

    Fix GitHub links. (google#50)

    These repos have moved to different GitHub user accounts or orgs.

commit 7fa4c26
Author: Victor Costan <costan@google.com>
Date:   Sun Dec 13 22:55:19 2020 -0800

    Update Travis CI config. (google#49)

commit a3d9e6d
Author: Victor Costan <costan@google.com>
Date:   Sun Dec 13 19:23:42 2020 -0800

    Updated third_party/ and Travis CI config. (google#48)
fanquake added a commit to bitcoin-core/crc32c-subtree that referenced this pull request Aug 30, 2024
21fc8ef Fix typo (google#59) (Dimitris Apostolou)
89f6984 Fix misspelled "Proccess" in comment (Munkybooty)
02e65f4 Bump deps (google#56) (Victor Costan)
b9d6e82 Fix Windows CI build. (google#54) (Victor Costan)
bbbb93a Switch CI to GitHub Actions (google#55) (Victor Costan)
d46cd17 Add clangd cache directory to .gitignore. (Victor Costan)

Pull request description:

  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.

Top commit has no ACKs.

Tree-SHA512: ca1cd5a085584d6a4ae65c5d83ae80db7bdd9ab251dc56fa899787dfa0b6fe0a60d32e89b5af5d066fa925a7120f02749aaa854e013f35c442f37a37a30caf23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants