-
Notifications
You must be signed in to change notification settings - Fork 96
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
Issue 4577 - Add GitHub actions #4625
Closed
+121
−60
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gcc-asan doesn't work with rust, so we can't use this. We need clang-asan to be consistent, especially so to match the correct llvm versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I will replace
gcc-asan
withclang-asan
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Firstyear, clang with
-fsanitize=address
fails to compile: https://github.com/vashirov/389-ds-base/runs/3554323865#step:6:3541Any ideas?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found that I filed an issue for this some time ago #4596
I feel that
clang-asan
would require more efforts than I expected, so I'd like to to keepgcc-asan
for now and addclang-asan
in a follow up PR.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't we change how lto was managed here to make it work? I'm building and using clang-asan just fine?
The issue with gcc-asan is that I've noticed more and more that gcc asan and rust's llvm backed asan aren't compatible and can not be mixed, which has led to failures to build. That's why I've been using clang now so that they both have the same llvm backend and asan versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, we didn't :( I couldn't make LTO builds with clang and then I switched to other things, forgetting about the issue.
I need some help from you regarding Rust + LTO, I will put the details in the related issue.
I haven't encountered any failures myself. Ideally I'd like to have the full matrix (
gcc
,gcc-asan
,clang
,clang-asan
), so that we can catch these build issues too. But I'm hitting GH actions limit of 256 jobs in the matrix. I have some ideas how to circumvent this with composable actions, will try it next week.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I'll look at the LTO issue later, but I think we can't do gcc-asan + rust at all. I've had so many conflicts between the two ASAN apis as they exist, and a bunch of false positives at this point, that I don't think it should be considered reliable.