Skip to content

Add mktemp_d#16

Merged
bors[bot] merged 19 commits intomatklad:masterfrom
azdavis:master
Mar 1, 2021
Merged

Add mktemp_d#16
bors[bot] merged 19 commits intomatklad:masterfrom
azdavis:master

Conversation

@azdavis
Copy link
Collaborator

@azdavis azdavis commented Feb 27, 2021

  • Use POSIX date invocations, don't use --iso option
    • This might have been part of what was making tests fail on macOS
    • I ran cargo t on my macOS machine and tests failed because macOS's default date doesn't have the --iso option
  • Add changelog entry for 0.1.7, 0.1.8 retroactively
  • Fix cp doc
    • It acts like shell's cp when dst is a directory
  • Allow acquiring many read locks on the same thread
    • Update the thread-local cache to track readers as well as the writer
  • Doc and test internal gsl more, note some caveats
  • Add mktemp_d, doc it
  • Update version and release notes
  • Ignore target files in vscode

Closes #12 , can also probably close #7 after this.

azdavis and others added 14 commits February 26, 2021 21:31
--iso is not POSIX, but format strings like +%Y-%m-%d are.[1]

Notably, --iso is not available on the default `date` for macOS as of
now.

[1]: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/date.html
Tried to piece things together from the commit log.
The previous formulation of Guard allowed (in the types, at least)
having Some(_) for both r_guard and w_guard. But this should never be
allowed.

It in fact was not ever constructed in this manner, and the fields are
private so we know it never will be. But let's forbid it with types.

Not totally sure if the drop impl is right, I'm not sure what reading
the r_guard did in the previous impl.
This deadlocked before the previous commit.
@azdavis azdavis mentioned this pull request Feb 27, 2021
Closed
Copy link
Owner

@matklad matklad left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks for looking into this! As a punishment for all the great stuff you've done, you are now a co-maintainer of this crate, so you can r+ yourself :)

src/fs.rs Outdated

impl Drop for TempDir {
fn drop(&mut self) {
rm_rf(&self.path).unwrap()
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
rm_rf(&self.path).unwrap()
let _ = rm_rf(&self.path);

or maybe if !std::thread::panicking() { .unwrap() }

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks like std is leery of panicking in destructors, so let's do the former. But maybe it would be nice to circle back and add a fn finish(self) -> Result<()> method to all the RAII guards if one wants to catch errors and not drop them on the floor.

@azdavis
Copy link
Collaborator Author

azdavis commented Feb 28, 2021

bors r+

@bors
Copy link
Contributor

bors bot commented Feb 28, 2021

🔒 Permission denied

Existing reviewers: click here to make azdavis a reviewer

@azdavis
Copy link
Collaborator Author

azdavis commented Feb 28, 2021

@matklad 🤔

@azdavis
Copy link
Collaborator Author

azdavis commented Mar 1, 2021

hm maybe i had to accept the crate owner invitation too? (just noticed and did)

bors r+

@bors
Copy link
Contributor

bors bot commented Mar 1, 2021

🔒 Permission denied

Existing reviewers: click here to make azdavis a reviewer

@matklad
Copy link
Owner

matklad commented Mar 1, 2021

@azdavis sorry, a form with two different fields bested me. Could you try again?

@azdavis
Copy link
Collaborator Author

azdavis commented Mar 1, 2021

bors r+

bors bot added a commit that referenced this pull request Mar 1, 2021
16: Add mktemp_d r=azdavis a=azdavis

- Use POSIX date invocations, don't use `--iso` option
  - This might have been part of what was making tests fail on macOS
  - I ran `cargo t` on my macOS machine and tests failed because macOS's default `date` doesn't have the `--iso` option
- Add changelog entry for 0.1.7, 0.1.8 retroactively
- Fix cp doc
  - It acts like shell's `cp` when `dst` is a directory
- Allow acquiring many read locks on the same thread
  - Update the thread-local cache to track readers as well as the writer
- Doc and test internal `gsl` more, note some caveats
- Add `mktemp_d`, doc it
- Update version and release notes
- Ignore `target` files in vscode

Closes #12 , can also probably close #7 after this.

Co-authored-by: Ariel Davis <ariel.z.davis@icloud.com>
Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
@azdavis
Copy link
Collaborator Author

azdavis commented Mar 1, 2021

seems like bors is stuck :/

@matklad
Copy link
Owner

matklad commented Mar 1, 2021

yeah, we observe the same on rust-analyzer's repo, rough luck

@bors
Copy link
Contributor

bors bot commented Mar 1, 2021

Timed out.

@azdavis
Copy link
Collaborator Author

azdavis commented Mar 1, 2021

might have been an issue with github? https://www.githubstatus.com/incidents/xn0sd2x4nd7f

bors r+

@bors
Copy link
Contributor

bors bot commented Mar 1, 2021

Build succeeded:

@bors bors bot merged commit 8c20f82 into matklad:master Mar 1, 2021
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.

No changelog entries for 0.1.7, 0.1.8

2 participants