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

corpus_merge.sh: make macOS compatible #17

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Crypt-iQ
Copy link
Collaborator

Tried running corpus_merge.sh and ran into some issues due to quirks of macOS.

  • When getting the number of inputs via wc -l, it would include whitespace. Now it gets piped to xargs to strip the whitespace.
  • When running the fuzz tests via measure_coverage, the current working directory was included along with the coverage bits. sed is used to strip the non-numerical characters

- xargs is needed to get rid of whitespace when getting the number of
  inputs

- sed is used to strip non-numerical characters that appeared during
  the grep (notably the current working directory)
@guggero
Copy link
Member

guggero commented Oct 17, 2023

cc @morehouse

@@ -103,7 +103,7 @@ mkdir "${CACHE_DIR}/${FUZZ_TARGET}"
coverage=0
if [[ -n $(ls "${DEST_DIR}") ]]; then
cp "${DEST_DIR}"/* "${CACHE_DIR}/${FUZZ_TARGET}/"
coverage=$(measure_coverage)
coverage=$(measure_coverage | sed 's/[^0-9]*//g')
Copy link
Contributor

Choose a reason for hiding this comment

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

What if the current directory has numbers in it?

Also, can we move the stripping of CWD into measure_coverage itself?

@lightninglabs-deploy
Copy link

@Crypt-iQ, remember to re-request review from reviewers when ready

@lightninglabs-deploy
Copy link

Closing due to inactivity

2 similar comments
@lightninglabs-deploy
Copy link

Closing due to inactivity

@lightninglabs-deploy
Copy link

Closing due to inactivity

@guggero
Copy link
Member

guggero commented Apr 17, 2024

!lightninglabs-deploy mute

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

Successfully merging this pull request may close these issues.

4 participants