Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Ci fuzz serde v6.2 #11763

Closed
wants to merge 2 commits into from
Closed

Conversation

catenacyber
Copy link
Contributor

@catenacyber catenacyber commented Sep 12, 2024

Link to ticket: https://redmine.openinfosecfoundation.org/issues/
None

Describe changes:

  • make CI and oss-fuzz green again by pinning old serde

#11756 with Cargo.toml change in sync with Cargo.lock change

#11741 still looks simpler/faster/better

#11764 is more complete

for oss-fuzz/cifuzz compatibility
As they use an old nightly compiler
because oss-fuzz does not support newer rust nightly
and newer serde does not support oss-fuzz old nightly
@jasonish
Copy link
Member

Probably should squash to one commit that updates the lock and the toml so you don't end up in a state where a commit has the lock and toml out of sync.

@catenacyber
Copy link
Contributor Author

Probably should squash to one commit that updates the lock and the toml so you don't end up in a state where a commit has the lock and toml out of sync.

#11741 without toml update still looks simpler/faster/better, just reverting the breaking change

Copy link

codecov bot commented Sep 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.54%. Comparing base (31bed10) to head (b5201f7).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #11763      +/-   ##
==========================================
- Coverage   82.62%   82.54%   -0.08%     
==========================================
  Files         919      919              
  Lines      248979   248979              
==========================================
- Hits       205722   205527     -195     
- Misses      43257    43452     +195     
Flag Coverage Δ
fuzzcorpus 60.32% <ø> (-0.54%) ⬇️
livemode 18.71% <ø> (+<0.01%) ⬆️
pcap 44.17% <ø> (+0.04%) ⬆️
suricata-verify 61.91% <ø> (+0.02%) ⬆️
unittests 58.99% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@victorjulien
Copy link
Member

Probably should squash to one commit that updates the lock and the toml so you don't end up in a state where a commit has the lock and toml out of sync.

#11741 without toml update still looks simpler/faster/better, just reverting the breaking change

We need both to be updated.

@catenacyber
Copy link
Contributor Author

Why so ?

@jasonish
Copy link
Member

I've squashed the commits here: #11766

Hopefully makes the FreeBSD builder happy if that's one of the issues.

@jasonish
Copy link
Member

Why so ?

While I don't think updating the toml is 100% required, we're pinning it back for a very specific reason, that should be in the toml so this change doesn't get "lost" in a future update to the toml.

@victorjulien
Copy link
Member

replaced by #11766

@catenacyber
Copy link
Contributor Author

that should be in the toml so this change doesn't get "lost" in a future update to the toml.

I hope this change will get "lost" ( because I hope suricata will be able to update to new serde and still have oss-fuzz )

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants