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.

Sign up for GitHub

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

Jump to bottom

fixes #1536

Merged
merged 3 commits into from
Aug 23, 2024
Merged

fixes #1536

merged 3 commits into from
Aug 23, 2024

Conversation

Byron
Copy link
Member

@Byron Byron commented Aug 22, 2024 •
edited
Loading

The problem is that with them, we don't notice anymore if the crate changes, because a dependency changes. That also means that older versions of the dependency may stay even though some other crates might pick up a newer version.

Ultimately, this will lead to drift and subtle incompatibilities.

We declare this breaking to enforce a proper re-release.

Note: Had to disable fuzzing again as it fails due to some incompatibility with serde and the nightly compiler that it appears to be using. I really have had it all tonight 😁

@Byron
fix!: remove all workspace dependencies
1757377 
The problem is that with them, we don't notice anymore if the crate changes,
because a dependency changes. That also means that older versions of the dependency
may stay even though some other crates might pick up a newer version.

Ultimately, this will lead to drift and subtle incompatibilities.

We declare this breaking to enforce a proper re-release.
EliahKagan referenced this pull request Aug 22, 2024
@Byron
Release gix v0.65.0, gix-fsck v0.5.0, gitoxide-core v0.40.0, gitoxide…
f218578 
… v0.38.0
@Byron
Looks like fuzzing is failing permanently again
3d90ab0 
Seems to be a mismatch between nightly compiler and `serde`.

````
EAD is now at a4b82c7 Merge 1757377 into b3ff033
.
2024-08-22 20:17:48,714 - root - INFO - repo_dir: /github/workspace/storage/gitoxide.
2024-08-22 20:17:48,719 - root - INFO - Docker container: 79524d8df2a2.
2024-08-22 20:17:48,720 - root - INFO - Building with address sanitizer.
2024-08-22 20:17:48,720 - helper - INFO - Running: docker run --privileged --shm-size=2g --platform linux/amd64 --rm -e FUZZING_ENGINE=libfuzzer -e CIFUZZ=True -e SANITIZER=address -e ARCHITECTURE=x86_64 -e FUZZING_LANGUAGE=rust -e OUT=/github/workspace/build-out --volumes-from 79524d8df2a2 gcr.io/oss-fuzz/gitoxide /bin/bash -c 'cd / && rm -rf /src/gitoxide/* && cp -r /github/workspace/storage/gitoxide /src && cd - && compile'.
/src/gitoxide
---------------------------------------------------------------
vm.mmap_rnd_bits = 28
Compiling libFuzzer to /usr/lib/libFuzzingEngine.a...  done.
---------------------------------------------------------------
CC=clang
CXX=clang++
CFLAGS=-O1 -fno-omit-frame-pointer -gline-tables-only -Wno-error=enum-constexpr-conversion -Wno-error=incompatible-function-pointer-types -Wno-error=int-conversion -Wno-error=deprecated-declarations -Wno-error=implicit-function-declaration -Wno-error=implicit-int -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -fsanitize=address -fsanitize-address-use-after-scope -fsanitize=fuzzer-no-link
CXXFLAGS=-O1 -fno-omit-frame-pointer -gline-tables-only -Wno-error=enum-constexpr-conversion -Wno-error=incompatible-function-pointer-types -Wno-error=int-conversion -Wno-error=deprecated-declarations -Wno-error=implicit-function-declaration -Wno-error=implicit-int -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -fsanitize=address -fsanitize-address-use-after-scope -fsanitize=fuzzer-no-link -stdlib=libc++
RUSTFLAGS=--cfg fuzzing -Zsanitizer=address -Cdebuginfo=1 -Cforce-frame-pointers
---------------------------------------------------------------
+ set -eox pipefail
+ export CARGO_BUILD_TARGET_DIR=/work/shared_cache
+ CARGO_BUILD_TARGET_DIR=/work/shared_cache
+++ readlink -f '{}'
++ find . -type d -name fuzz -exec dirname '/src/gitoxide/{}' ';'
+ FUZZ_CRATE_DIRS='/src/gitoxide/./gix-commitgraph
/src/gitoxide/./gix-revision
/src/gitoxide/./gix-refspec
/src/gitoxide/./gix-pathspec
/src/gitoxide/./gix-object
/src/gitoxide/./gix-ref
/src/gitoxide/./gix-attributes
/src/gitoxide/./gix-config
/src/gitoxide/./gix-config-value
/src/gitoxide/./gix-url
/src/gitoxide/./gix-date'
+ for CRATE_DIR in ${FUZZ_CRATE_DIRS[@]}
+ echo 'Building crate: /src/gitoxide/./gix-commitgraph'
+ cd /src/gitoxide/./gix-commitgraph
+ cargo fuzz build -O --debug-assertions
Building crate: /src/gitoxide/./gix-commitgraph
    Updating crates.io index
 Downloading crates ...
  Downloaded derive_arbitrary v1.3.2
  Downloaded arbitrary v1.3.2
  Downloaded jobserver v0.1.32
  Downloaded faster-hex v0.9.0
  Downloaded shlex v1.3.0
  Downloaded thiserror-impl v1.0.63
  Downloaded thiserror v1.0.63
  Downloaded unicode-ident v1.0.12
  Downloaded memchr v2.7.4
  Downloaded syn v2.0.75
  Downloaded bstr v1.10.0
  Downloaded libc v0.2.158
  Downloaded libfuzzer-sys v0.4.7
  Downloaded serde v1.0.208
  Downloaded quote v1.0.36
  Downloaded proc-macro2 v1.0.86
  Downloaded memmap2 v0.9.4
  Downloaded cc v1.1.13
  Downloaded sha1_smol v1.0.1
  Downloaded once_cell v1.19.0
  Downloaded anyhow v1.0.86
   Compiling proc-macro2 v1.0.86
   Compiling libc v0.2.158
   Compiling unicode-ident v1.0.12
   Compiling serde v1.0.208
   Compiling thiserror v1.0.63
   Compiling shlex v1.3.0
error[E0658]: `#[diagnostic]` attribute name space is experimental
   --> /rust/registry/src/index.crates.io-6f17d22bba15001f/serde-1.0.208/src/de/mod.rs:537:5
    |
537 |     diagnostic::on_unimplemented(
    |     ^^^^^^^^^^
    |
    = note: see issue #111996 <rust-lang/rust#111996> for more information
    = help: add `#![feature(diagnostic_namespace)]` to the crate attributes to enable
    = note: this compiler was built on 2024-02-11; consider upgrading it if it is out of date

error[E0658]: `#[diagnostic]` attribute name space is experimental
   --> /rust/registry/src/index.crates.io-6f17d22bba15001f/serde-1.0.208/src/ser/mod.rs:220:5
    |
220 |     diagnostic::on_unimplemented(
    |     ^^^^^^^^^^
    |
    = note: see issue #111996 <rust-lang/rust#111996> for more information
    = help: add `#![feature(diagnostic_namespace)]` to the crate attributes to enable
    = note: this compiler was built on 2024-02-11; consider upgrading it if it is out of date

   Compiling quote v1.0.36
   Compiling syn v2.0.75
   Compiling jobserver v0.1.32
   Compiling cc v1.1.13
   Compiling memchr v2.7.4
For more information about this error, try `rustc --explain E0658`.
The following warnings were emitted during compilation:

warning: [email protected]: cargo:rustc-check-cfg requires -Zcheck-cfg flag
warning: [email protected]: cargo:rustc-check-cfg requires -Zcheck-cfg flag
warning: [email protected]: cargo:rustc-check-cfg requires -Zcheck-cfg flag
warning: [email protected]: cargo:rustc-check-cfg requires -Zcheck-cfg flag
warning: [email protected]: cargo:rustc-check-cfg requires -Zcheck-cfg flag
warning: [email protected]: cargo:rustc-check-cfg requires -Zcheck-cfg flag
warning: [email protected]: cargo:rustc-check-cfg requires -Zcheck-cfg flag
warning: [email protected]: cargo:rustc-check-cfg requires -Zcheck-cfg flag
warning: [email protected]: cargo:rustc-check-cfg requires -Zcheck-cfg flag
warning: [email protected]: cargo:rustc-check-cfg requires -Zcheck-cfg flag
warning: [email protected]: cargo:rustc-check-cfg requires -Zcheck-cfg flag
warning: [email protected]: cargo:rustc-check-cfg requires -Zcheck-cfg flag

error: could not compile `serde` (lib) due to 2 previous errors
warning: build failed, waiting for other jobs to finish...
Error: failed to build fuzz script: ASAN_OPTIONS="detect_odr_violation=0" RUSTFLAGS="-Cpasses=sancov-module -Cllvm-args=-sanitizer-coverage-level=4 -Cllvm-args=-sanitizer-coverage-inline-8bit-counters -Cllvm-args=-sanitizer-coverage-pc-table -Cllvm-args=-sanitizer-coverage-trace-compares --cfg fuzzing -Clink-dead-code -Zsanitizer=address -Cllvm-args=-sanitizer-coverage-stack-depth -Cdebug-assertions -C codegen-units=1 --cfg fuzzing -Zsanitizer=address -Cdebuginfo=1 -Cforce-frame-pointers" "cargo" "build" "--manifest-path" "/src/gitoxide/gix-commitgraph/fuzz/Cargo.toml" "--target" "x86_64-unknown-linux-gnu" "--release" "--config" "profile.release.debug=true" "--bins"
2024-08-22 20:17:58,803 - root - ERROR - Building fuzzers failed.
2024-08-22 20:17:58,803 - root - ERROR - Error building fuzzers for (commit: a4b82c7, pr_ref: refs/pull/1536/merge).
````
@EliahKagan
Copy link
Member

Does this change have any bearing on the feasibility of #1510? (I'm asking because I notice that's still open, even though the comment discussion suggests it may not be something that can or should be done.)

@EliahKagan
Copy link
Member

It looks like the fuzzing failure may have been temporary, since I ran it in my fork, both on main and at 1757377 from this PR, a few times each, and it ran with no problems:

Maybe rerunning the failing job here would work as well, and the removal in 3d90ab0 would not be needed.

@Byron
Copy link
Member Author

Byron commented Aug 23, 2024 •
edited
Loading

Does this change have any bearing on the feasibility of #1510? (I'm asking because I notice that's still open, even though the comment discussion suggests it may not be something that can or should be done.)

I think that's unrelated.

Thanks for the hint about fuzzing - I will try again.

Edit: It still fails for me.

@Byron Byron merged commit 46cd1ae into main Aug 23, 2024
18 checks passed
@Byron Byron deleted the fixes branch August 23, 2024 05:57
@EliahKagan
Copy link
Member

EliahKagan commented Aug 23, 2024 •
edited
Loading

Thanks for the hint about fuzzing - I will try again.

Edit: It still fails for me.

That's odd, and I wonder if it is somehow influenced by which event trigger is used (since while I ran it repeatedly, I triggered the workflow via workflow_dispatch from the Actions tab, not via pull_request). I'll see if I can figure anything out.

@EliahKagan EliahKagan mentioned this pull request Aug 23, 2024
Closed
EliahKagan added a commit to EliahKagan/gitoxide that referenced this pull request Sep 15, 2024
@EliahKagan
Try manully specifying old serde{,_derive,_json} in Cargo.toml
aafa8b1 
As before GitoxideLabs#1536.

I'm not sure CI fuzzing actually uses locked dependencies, though.
@EliahKagan EliahKagan mentioned this pull request Sep 15, 2024
Merged
EliahKagan added a commit to EliahKagan/oss-fuzz that referenced this pull request Sep 18, 2024
@EliahKagan
gitoxide: Use updated nightly to fix serde error
e8dd8ab 
Since around GitoxideLabs/gitoxide#1536, fuzzing
is broken for `gitoxide` due to an error related to `serde`. As
shown there and in GitoxideLabs/gitoxide#1596, the
error is:

    error[E0658]: `#[diagnostic]` attribute name space is experimental
       --> /rust/registry/src/index.crates.io-6f17d22bba15001f/serde-1.0.210/src/de/mod.rs:536:5
        |
    536 |     diagnostic::on_unimplemented(
        |     ^^^^^^^^^^
        |
        = note: see issue #111996 <rust-lang/rust#111996> for more information
        = help: add `#![feature(diagnostic_namespace)]` to the crate attributes to enable
        = note: this compiler was built on 2024-02-11; consider upgrading it if it is out of date

Since rust-lang/rust#111996 is closed as
completed, and similar errors appear to have been fixed in oss-fuzz
for other projects by using the latest nightly toolchain, this
makes the same change for `gitoxide` as was made in

- google#12404 for `starlark-rust`
- google#12409 for `rhai`

See also google#12410.
@EliahKagan EliahKagan mentioned this pull request Sep 18, 2024
Merged
EliahKagan added a commit to EliahKagan/oss-fuzz that referenced this pull request Sep 18, 2024
@EliahKagan
gitoxide: Use updated nightly to fix serde error
facfab1 
Since around GitoxideLabs/gitoxide#1536, fuzzing
is broken for `gitoxide` due to an error related to `serde`. As
shown there and in GitoxideLabs/gitoxide#1596, the
error is:

    error[E0658]: `#[diagnostic]` attribute name space is experimental
       --> /rust/registry/src/index.crates.io-6f17d22bba15001f/serde-1.0.210/src/de/mod.rs:536:5
        |
    536 |     diagnostic::on_unimplemented(
        |     ^^^^^^^^^^
        |
        = note: see issue #111996 <rust-lang/rust#111996> for more information
        = help: add `#![feature(diagnostic_namespace)]` to the crate attributes to enable
        = note: this compiler was built on 2024-02-11; consider upgrading it if it is out of date

Since rust-lang/rust#111996 is closed as
completed, and similar errors appear to have been fixed in oss-fuzz
for other projects by using the latest nightly toolchain, this
makes the same change for `gitoxide` as was made in:

- google#12404 for `starlark-rust`
- google#12409 for `rhai`

See also :

- google#12410
- serde-rs/serde#2770
DavidKorczynski pushed a commit to google/oss-fuzz that referenced this pull request Sep 18, 2024
@EliahKagan
gitoxide: Use updated nightly to fix serde error (#12512)
a06f052 
Since around GitoxideLabs/gitoxide#1536, fuzzing is
broken for `gitoxide` due to an error related to `serde`. As shown there
and in GitoxideLabs/gitoxide#1596, the error is:

    error[E0658]: `#[diagnostic]` attribute name space is experimental
-->
/rust/registry/src/index.crates.io-6f17d22bba15001f/serde-1.0.210/src/de/mod.rs:536:5
        |
    536 |     diagnostic::on_unimplemented(
        |     ^^^^^^^^^^
        |
= note: see issue #111996
<rust-lang/rust#111996> for more information
= help: add `#![feature(diagnostic_namespace)]` to the crate attributes
to enable
= note: this compiler was built on 2024-02-11; consider upgrading it if
it is out of date

Since rust-lang/rust#111996 is closed as
completed, and similar errors appear to have been fixed in oss-fuzz for
other projects by using the latest nightly toolchain, this makes the
same change for `gitoxide` as was made in:

- #12404 for `starlark-rust`
- #12409 for `rhai`

See also:

- #12410
- serde-rs/serde#2770

cc @Byron
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Byron @EliahKagan