pantsbuild #development

Channels

welcome

github-notifications

development

witty-crayon-22786

03/27/2017, 7:00 PM

then, to mutate the contents, you'd do

witty-crayon-22786

03/27/2017, 7:01 PM

Copy code

let mut tg = self.temp_generator.lock().unwrap();
tg.1 = tg.1 + 1;
..

approximately

witty-crayon-22786

03/27/2017, 7:02 PM

but if there is a crate that supports just creating temp files, that might be easier.

rough-minister-58256

03/27/2017, 7:12 PM

I looked at

tempfile

, but that actually creates the

File

object on your behalf (and requires external parent dir creation) which seemed a little less desirable to plumb than the tempdir + atomic # -> name ->

file::File::create

we have now

witty-crayon-22786

03/27/2017, 7:21 PM

yea, either could work.

witty-crayon-22786

03/27/2017, 7:54 PM

@fancy-queen-20734 : hm, so i know i said that area of the code wouldn't be changing, but: https://github.com/pantsbuild/pants/pull/4381 should actually help with https://github.com/pantsbuild/pants/pull/4371 ... if you have a chance today, would appreciate a review. it just shuffles things... no semantic changes.

fancy-queen-20734

03/27/2017, 8:48 PM

@witty-crayon-22786 🚢

witty-crayon-22786

03/27/2017, 8:49 PM

thanks

rough-minister-58256

03/27/2017, 10:02 PM

how gross would it be to just recreate the original TempDir on access? it’s super simple and only requires a concurrent safe mkdir. while possible that paths could collide concurrently, it’s not probable due to random string in the path.

rough-minister-58256

03/27/2017, 10:02 PM

I lifted a better concurrent-safe-mkdir from rustc that minimizes syscalls w/o retries

witty-crayon-22786

03/27/2017, 10:03 PM

@rough-minister-58256 : i figure you have to do it for the parent directory, anyway... right?

rough-minister-58256

03/27/2017, 10:03 PM

yeah

rough-minister-58256

03/27/2017, 10:04 PM

so really its the diff between

.pants.d/snapshots/.tmp/snapshots.$RANDOM_STR

going away and either recreating the same exact path or with a replacement approach create

.pants.d/snapshots/.tmp/snapshots.$NEXT_RANDOM_STR

etc

rough-minister-58256

03/27/2017, 10:04 PM

not likely that other concurrent tmp dir creators will key to the same tmp path since it contains a random string

rough-minister-58256

03/27/2017, 10:04 PM

and would avoid the whole need for interior mutability

witty-crayon-22786

03/27/2017, 10:09 PM

@rough-minister-58256 : the concurrent creation is still necessary though, because you're racing other threads in the same process as well

rough-minister-58256

03/27/2017, 10:09 PM

yeah, concurrent creation is in place

witty-crayon-22786

03/27/2017, 10:09 PM

rough-minister-58256

03/27/2017, 10:11 PM

the q is: would it be ok to just

safe_create_dir_all(self.snapshot_tmpdir.path())

rather than ~`self.snapshot_tmpdir = TempDir::new_in(…)` via a mutex.

witty-crayon-22786

03/27/2017, 10:11 PM

depends on the implementation details of

TempDir

. but probably.

witty-crayon-22786

03/27/2017, 10:12 PM

...er. hm.

rough-minister-58256

03/27/2017, 10:12 PM

I think its fine, but wouldn’t provide the exact same concurrent guarantees that

TempDir::new

etc provide w/ retries and such - but the probability of keying back to the exact same

TempDir

path is low afaict.

witty-crayon-22786

03/27/2017, 10:12 PM

blargh. as you said, another process could try to create the same one.

rough-minister-58256

03/27/2017, 10:12 PM

https://doc.rust-lang.org/tempdir/src/tempdir/lib.rs.html#184

witty-crayon-22786

03/27/2017, 10:12 PM

ok, i'll say again: really seems like tempfiles might be the way to go.

rough-minister-58256

03/27/2017, 10:13 PM

could try - but isnt likely to try.

witty-crayon-22786

03/27/2017, 10:13 PM

you mention that the need to create the parent directory is an issue, but that just seems like a thing we already need to do.

witty-crayon-22786

03/27/2017, 10:13 PM

and also, that would mean directory creation could be fully lazy

witty-crayon-22786

03/27/2017, 10:14 PM

~two syscalls to start writing a snapshot each time, but oh well.

rough-minister-58256

03/27/2017, 10:14 PM

you mention that the need to create the parent directory is an issue