Add [limited] support for WASI

[RReverser]

WASI doesn't have a concept of system-wide temporary directory, but at least there is no reason why new_in methods with explicit paths should fail.

This PR implements support for such methods.

[Stebalien]

Thanks!

[Stebalien]

Also, if there is a good way to test this that would be nice. I've run introduced an unfortunate number of bugs in platforms not tested in CI.

[RReverser]

Also, if there is a good way to test this that would be nice. I've run introduced an unfortunate number of bugs in platforms not tested in CI.

So that's kinda the problem - testing itself is easy, but I was surprised to find that all the currently written tests use methods based on std::env::temp_dir().

I can write a test or two specifically for new_in + WASI, but then I'd still need to somehow disable all the other existing tests, as they definitely fail on WASI, and things get ugly plus coverage stays pretty poor anyway, so I'm not sure if it's worth it...

[Stebalien]

So that's kinda the problem - testing itself is easy, but I was surprised to find that all the currently written tests use methods based on std::env::temp_dir().

Take a look in the tests directory.

I can write a test or two specifically for new_in + WASI, but then I'd still need to somehow disable all the other existing tests, as they definitely fail on WASI, and things get ugly plus coverage stays pretty poor anyway, so I'm not sure if it's worth it...

You should be able to disable the tempdir tests using a #![cfg(...)] at the top of the test file, IIRC (I haven't done this kind of thing in a while).

Honestly, coverage isn't really something I'm too concerned about here. I just want to make sure it compiles and basically works. Compiles is probably a high enough bar if you can get that tested in CI.

[RReverser]

Take a look in the tests directory.

I did, but I guess I missed test_tempdir:

tempfile/tests/tempdir.rs

Lines 44 to 52 in 57072e4

	fn test_tempdir() {
	let path = {
	let p = t!(Builder::new().prefix("foobar").tempdir_in(&Path::new(".")));
	let p = p.path();
	assert!(p.to_str().unwrap().contains("foobar"));
	p.to_path_buf()
	};
	assert!(!path.exists());
	}

Aside from that, all the other tests seem to rely on std::env::temp_dir() either directly or indirectly.

Compiles is probably a high enough bar if you can get that tested in CI.

Ah, that should be trivial. I'll look into that tomorrow.

[Stebalien]

Aside from that, all the other tests seem to rely on std::env::temp_dir() either directly or indirectly.

Oh, sorry, I misread. I thought you were saying that only temporary directories were tested.

Yeah, I see the issue. Eventually it would probably be a good idea to just pick a sane default for std::env::temp_dir(), but just compiling in CI is likely enough for now.

[RReverser]

Ok I've added two jobs - one for WASI on Rust 1.40.0 on Linux, and one for WASI on nightly Rust with nightly feature on Linux.

I believe those should be sufficient, as they cover MSRV as well as nightly build, and we don't want to test all possible combinations as that quickly explodes number of jobs and significantly increases CI time for little benefit.

One more thing I noticed is that AppVeyor shows up on Github PR checks and commits, but Travis doesn't - perhaps integration is not correctly enabled in repo settings?

[RReverser]

Eventually it would probably be a good idea to just pick a sane default for std::env::temp_dir(), but just compiling in CI is likely enough for now.

As for this - I was thinking about potential solutions, and I wonder if a better option would be to expose API to customize root temp dir instead? It would not only avoid hardcoding a default that might not match WASI user's expectations, but also potentially help users on other systems too in case they want to choose a custom root temporary dir (e.g. this could be used as a workaround in #112).

I'm not ready to take on this in a follow-up PR (not too much time), just sharing an idea :)

[Stebalien]

As for this - I was thinking about potential solutions, and I wonder if a better option would be to expose API to customize root temp dir instead? It would not only avoid hardcoding a default that might not match WASI user's expectations, but also potentially help users on other systems too in case they want to choose a custom root temporary dir (e.g. this could be used as a workaround in #112).

Ideally, they'd set the default through the system itself (e.g., TMPDIR). I prefer to avoid providing global "setters" as they tend to be abused.

[Stebalien]

Thanks!

[RReverser]

Ideally, they'd set the default through the system itself (e.g., TMPDIR).

Yeah that's an option too, but still a little concerning because WASI intentionally doesn't try to emulate POSIX, and intentionally tries not to rely on any environment variables.

It's still possible for separate crates to choose to rely on those, of course, but a bit worried that it might create inconsistency between parts of the ecosystem...

Maybe a crate-specific env var like TEMPFILE_RS_DIR would be fine though.

[RReverser]

Although... maybe it wouldn't be too bad. I guess TMPDIR is a fairly explicit signal, especially if documented in the crate.

I wish this was defined at WASI level or Rust stdlib level to ensure consistency, but given popularity of this crate, I suppose that should be enough.

[Stebalien]

At the moment, I think it's best to wait. I assume the WASI runtime environment will eventually pick something.

[RReverser]

One more thing I noticed is that AppVeyor shows up on Github PR checks and commits, but Travis doesn't - perhaps integration is not correctly enabled in repo settings?

Looks like this made it easy to miss one compilation issue with WASI in latest commit... I'll make a follow-up fix, but I think integration to see checks in Github could be helpful anyway :) https://travis-ci.org/github/Stebalien/tempfile/builds/757241191

[Stebalien]

Unfortunately, travis is switching from travis-ci.org to travis-ci.com. Along with that, they're demanding access to all repos, public and private.

In other news, I'm going to be looking for a new CI service.

[RReverser]

FWIW I've been starting new projects as well as migrating some old ones to Github Actions, and it's been a breeze so far. A lot faster than Travis, too.

[RReverser]

Oh, and they have Windows support, definitely faster than AppVeyor as well :)

[Stebalien]

Thanks!