search

pkgxdev / libpkgx

`import`… but with pkging powers
https://npmjs.com/libpkgx
Apache License 2.0
62 stars 11 forks source link

fix: rename TEA_PREFIX to TEA_DIR #43

Closed branchvincent closed 10 months ago

branchvincent commented 11 months ago

Renames TEA_PREFIX -> TEA_DIR

The v1 cli warns about this change:

$ tea --prefix
deprecated: TEA_PREFIX has been renamed TEA_DIR, migrate before v1.0.0+gold

But lib continues to install to TEA_PREFIX. Also

mxcl commented 11 months ago

Thanks for this, as you can tell our revlock is pretty bad and thus this will not build. I have fixes locally that I intend to apply for v1 which also do this.

The parallelization is perhaps unwise in the current state of these tests since they may operate on the same TEA_DIR, but I have fixed that in my local branch also so we could reapply that then.

mxcl commented 11 months ago

k this is now done, the other changes are welcome. I can probs tease them out.

I believe other changes I made for 0.14 make parallelization safe too, so I'll check.

mxcl commented 10 months ago

Run tests on main to help catch those failures sooner Run tests in parallel, reducing the runtime from 60s -> 40s (on my machine)

How do you make these changes? I can't see the code that does this in your PR.

branchvincent commented 10 months ago

it was just deno test --parallel in 3623f12756a33c865e9149a4d4e3c9f3eee5882d, but I reverted that once I saw the issues with parallel downloads (I believe writes to TEA_PREFIX had file locking but not writes to TEA_CACHE_DIR, which impacted the tests since the prefix was random for each test but all shared the user's cache)

branchvincent commented 10 months ago

and then ec6c87f38d55f99f9bdae7c935589fa830409421 just runs ci on pushes to main

mxcl commented 10 months ago

but I reverted that once I saw the issues with parallel downloads (I believe writes to TEA_PREFIX had file locking but not writes to TEA_CACHE_DIR

This is true in that the locking happens on the prefix and not the cache. I didn't think about this when I made the cache directory independently configurable. Whoops. Will need to think about that.

All the same all the tests now operate independently and almost entirely without any dependency on the pantry or pkg store so parallel should work just fine. I'll try it out.

Edit: I mean after #48 is merged anyway.

branchvincent commented 10 months ago

Awesome, thanks for integrating those changes. I'll go ahead and close this

mxcl commented 10 months ago

Thank you for the contribution!