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

try to keep libzstd.a "as is" once created #2438

Merged
merged 7 commits into from
Dec 22, 2020
Merged

try to keep libzstd.a "as is" once created #2438

merged 7 commits into from
Dec 22, 2020

Conversation

Cyan4973
Copy link
Contributor

@Cyan4973 Cyan4973 commented Dec 21, 2020

to improve compatibility with parallel build scenarios such as make -j allmost.
Presumed to fix #2436

to be compatible with scenarios such as
`make -j allmost`
Copy link
Contributor

@terrelln terrelln left a comment

Choose a reason for hiding this comment

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

Tests are failing

@Cyan4973
Copy link
Contributor Author

All tests fixed.
Remaining ones are failing independently of this PR.
32-bit test is fixed in #2440.
Fuzzing + msan has been failing for a while now,
I may disable it on short term so that we stop considering "normal" to have red failures in CI.

@Cyan4973
Copy link
Contributor Author

Actually, disabling fuzzing + msan seems more difficult than anticipated,
as it seems to be a runtime issue (time out),
but the script controllling all fuzzing jobs seems controlled from oss-fuzz :
https://github.com/facebook/zstd/blob/dev/.github/workflows/main.yml#L18 ,
so there is no obvious way to fix it, nor to opt out selectively from msan tests (but keep the rest running)
and I'm reluctant about removing all oss-fuzz tests on the ground that the msan variant is flackey.

@terrelln
Copy link
Contributor

I'll open an issue on oss-fuzz about the msan failure.

@terrelln
Copy link
Contributor

It looks like check-32bit test is failing, but that seems unrelated to this PR I think.

@terrelln
Copy link
Contributor

google/oss-fuzz#4879

@Cyan4973 Cyan4973 merged commit d4f7d75 into dev Dec 22, 2020
@evverx
Copy link
Contributor

evverx commented Dec 28, 2020

the script controllling all fuzzing jobs seems controlled from oss-fuzz :
https://github.com/facebook/zstd/blob/dev/.github/workflows/main.yml#L18 ,
so there is no obvious way to fix it, nor to opt out selectively from msan tests (but keep the rest running)
and I'm reluctant about removing all oss-fuzz tests on the ground that the msan variant is flackey.

I think it should be possible to remove "memory" from the "sanitizer" list to turn off only Msan there:

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 0e14345..5e5aae1 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -6,7 +6,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        sanitizer: [address, undefined, memory]
+        sanitizer: [address, undefined]
     steps:
     - name: Build Fuzzers (${{ matrix.sanitizer }})
       id: build

Though I hope the issue will be fixed soon.

@Cyan4973
Copy link
Contributor Author

That's a good point @evverx ,
we should probably do that while waiting for the issue to get fixed on ossfuzz side.

@Cyan4973 Cyan4973 deleted the makej branch May 4, 2021 06:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parallel build of v1.4.8 can fail
4 participants