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

Eliminate "text file busy" errors #140

Merged
merged 2 commits into from
Aug 27, 2024
Merged

Conversation

smoelius
Copy link
Contributor

@smoelius smoelius commented Aug 26, 2024

While running some tests involving Foundry, I have encountered "text file busy" errors, like described in #16.

This PR is currently organized as two commits:

  • Add can_install_while_solc_is_running test - intentionally fails with "text file busy"
  • Eliminate "test file busy" errors - installs solc to a temporary file and then renames it, so that the previous test passes

Nits are welcome.

EDIT: I'll be happy to fix the typo in the commit message, once you've had a chance to review. Fixed.

@smoelius smoelius changed the title Eliminate "test file busy" errors Eliminate "text file busy" errors Aug 26, 2024
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

lgtm, pending @DaniPopes

Comment on lines 162 to 164
let named_temp_file = tempfile::NamedTempFile::new_in(data_dir())?;
let (mut f, temp_path) = named_temp_file.into_parts();
Copy link
Member

Choose a reason for hiding this comment

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

ah okay, all this does is write to a new tmp file and then move it to the correct location?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, that just unpacks the NamedTempFile into a File and a TempPath.

The renaming occurs here:

temp_path.persist(&solc_path)?;

Some additional context:

@smoelius
Copy link
Contributor Author

FYI, I'm going to try to fix the Windows failure.

@smoelius smoelius force-pushed the text-file-busy branch 2 times, most recently from d163426 to e7daf1a Compare August 27, 2024 00:17
@smoelius
Copy link
Contributor Author

smoelius commented Aug 27, 2024

There were two Windows problems:

  • The new test must use where (not which) to find sleep.
  • Apparently, Windows does not allow a file to be renamed to a path that is opened. So the file at solc_path must be renamed before the new file can be renamed to solc_path.

Both of these should be fixed now.

@mattsse
Copy link
Member

mattsse commented Aug 27, 2024

clippy unrelated

@mattsse mattsse merged commit 90eafe7 into alloy-rs:master Aug 27, 2024
7 of 8 checks passed
@smoelius smoelius deleted the text-file-busy branch August 27, 2024 10:54
@smoelius
Copy link
Contributor Author

Thanks!

@mattsse
Copy link
Member

mattsse commented Aug 27, 2024

thank you 🫡

smoelius added a commit to trailofbits/necessist that referenced this pull request Aug 28, 2024
smoelius added a commit to trailofbits/necessist that referenced this pull request Aug 28, 2024
github-merge-queue bot pushed a commit to trailofbits/necessist that referenced this pull request Aug 28, 2024
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.

2 participants