Skip to content

Revert unintended CR/LF changes to non-3C files that originated in 277d84a5841634783e7d3752707e2a1e4e57a930. #961

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

Closed
secure-sw-dev-bot opened this issue Jan 16, 2022 · 8 comments

Comments

@secure-sw-dev-bot
Copy link

This issue was copied from checkedc/checkedc-clang#962


We first submitted them in PR #837, but Mandeep noticed
(checkedc/checkedc-clang#837 (review))
and the unintended changes were removed from that PR in
74bfcafad6b194bec68c30e04063aea5591a7f7a. However, when the squash of
PR #837 was merged with the original commits in
cfc998e, the unintended changes were
incorrectly retained. They got submitted again in the next 3C PR
(#891), and no one noticed that time.

@secure-sw-dev-bot
Copy link
Author

Comment from @mgrang:

LGTM. @mattmccutchen-cci Could you please verify if the validations are passing on Linux and Windows with this change?

@secure-sw-dev-bot
Copy link
Author

Comment from @mattmccutchen-cci:

@mattmccutchen-cci Could you please verify if the validations are passing on Linux and Windows with this change?

I could run the 3C regression tests, but I'm not familiar with how to run your full set of tests (which presumably includes the 3C regression tests). Can you just run them as you would on any other PR?

@secure-sw-dev-bot
Copy link
Author

Comment from @mgrang:

@mattmccutchen-cci Could you please verify if the validations are passing on Linux and Windows with this change?

I could run the 3C regression tests, but I'm not familiar with how to run your full set of tests (which presumably includes the 3C regression tests). Can you just run them as you would on any other PR?

You can run ninja check-all on Linux and Windows.

@secure-sw-dev-bot
Copy link
Author

Comment from @mattmccutchen-cci:

ninja check-all passes on my revert-unintended-crlf-changes-test-merge in our Linux environment. check-all failed to build in our Windows X64 environment with Visual Studio 2019 Community 16.8.4 due to this problem; the same problem occurs without this PR. (check-clang-3c is not affected.) Whatever we blame for the problem, it supposedly no longer occurs with LLVM 11.

I thought I've seen you trigger some process that runs the tests in your environment and posts GitHub status checks on the PR, and I assume that for whatever reason, it isn't affected by the build failure. Can you just use that process?

@secure-sw-dev-bot
Copy link
Author

Comment from @mgrang:

ninja check-all passes on my revert-unintended-crlf-changes-test-merge in our Linux environment. check-all failed to build in our Windows X64 environment with Visual Studio 2019 Community 16.8.4 due to this problem; the same problem occurs without this PR. (check-clang-3c is not affected.) Whatever we blame for the problem, it supposedly no longer occurs with LLVM 11.

I thought I've seen you trigger some process that runs the tests in your environment and posts GitHub status checks on the PR, and I assume that for whatever reason, it isn't affected by the build failure. Can you just use that process?

We discussed the Windows X64 build issue with @mwhicks1 and he says you should be able to build Windows X64 version without issues.

@secure-sw-dev-bot
Copy link
Author

Comment from @mwhicks1:

@mgrang What I said is what you've quoted in the issue: We can successful fun check-clang-3c on Windows X64. This build target requires clang, 3c, and other tools. So in this sense we can "build Windows X64 version."

I don't know what's in check-all that's the problem, but it's not an inability to construct Checked C's clang on Windows X64.

@secure-sw-dev-bot
Copy link
Author

Comment from @mgrang:

I will go ahead and merge this PR. I will let you know in case our ADO validations break after the merge.

@secure-sw-dev-bot
Copy link
Author

Comment from @mattmccutchen-cci:

Thanks @mgrang! Hopefully we get these testing process issues sorted out over time.

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

No branches or pull requests

1 participant