JuliaIO / Tar.jl

TAR files: create, list, extract them in pure Julia
MIT License
79 stars 19 forks source link

Add regression test for permissions in `Tar.create` #89

Closed giordano closed 3 years ago

giordano commented 3 years ago

Before switching to Tar.jl, for some time Pkg.PlatformEngines.package produced tarballs with wrong permissions under a weird condition: a directory had a symlink pointing to it, see https://github.com/JuliaLang/Pkg.jl/issues/2185 and https://github.com/JuliaPackaging/Yggdrasil/issues/1969. This PR adds a regression test to prevent Tar.jl from doing something similarly silly.

Close https://github.com/JuliaLang/Pkg.jl/issues/2185, close https://github.com/JuliaLang/Pkg.jl/pull/2274.

codecov[bot] commented 3 years ago

Codecov Report

Merging #89 (cc4c7cc) into master (0ba9683) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #89   +/-   ##
=======================================
  Coverage   96.37%   96.37%           
=======================================
  Files           4        4           
  Lines         634      634           
=======================================
  Hits          611      611           
  Misses         23       23           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 0ba9683...cc4c7cc. Read the comment docs.

StefanKarpinski commented 3 years ago

Not sure why this test is failing on nightly. Failing to detect the executable bit correctly should have been fixed already since we use Sys.isexecutable and that should now be reliable on Windows.

giordano commented 3 years ago

Ouch, it's failing because the version check is wrong: VERSION <= v"1.5" is false for v1.5.3. I guess I need to find the exact version where the isexecutable function was fixed and use that for the check

StefanKarpinski commented 3 years ago

I would just change the test to VERSION < v"1.6-" instead. That's good enough — it's not like we're going to start testing this on older versions of 1.6. This is exactly what those v"1.6-" and v"1.6+" versions are for.

giordano commented 3 years ago

Oh, good to know, but too late :smile: