Run Pkg.precompile before tests in same julia setup as tests for correct caching

[IanButterworth]

Required to avoid double-precompilation when native code caching is enabled

[staticfloat]

looks reasonable to me!

[KristofferC]

I don't get why this is needed. If the tests start with the correct project, why isn't just a Pkg.precompile enough?

[IanButterworth]

I assumed that importing Pkg into the test process might be breaking, so I left that untouched

[KristofferC]

Didn't we say we needed to disable auto precompile on CI? Otherwise, you will precompile once when the deps are installed (which has the wrong flags) and then once before the tests (with different flags so will be a full re-precompile).

[IanButterworth]

Didn't we say we needed to disable auto precompile on CI?

Correct. Note that it was that way already and this doesn't change that.

[KristofferC]

Oh, I didn't know that. Nice :).

Then I think this should be good. Would be good to confirm this whole PR by setting ENV["CI"] = "true, and running a Pkg.instantiate() + Pkg.test() on some package with the code caching PR and see that things work as expected.

[IanButterworth]

I think this shows it's working.

All 19 dependencies had to be re-precompiled before tests ran.

shell> rm -rf ~/.julia/compiled/v1.10/

(jl_zoE1I6) pkg> st
Status `/private/var/folders/_6/1yf6sj0950vcg4t91m9ltb5w0000gn/T/jl_zoE1I6/Project.toml`
  [336ed68f] CSV v0.10.8

(jl_zoE1I6) pkg> precompile
Precompiling environment...
  19 dependencies successfully precompiled in 53 seconds

(jl_zoE1I6) pkg> test CSV
     Testing CSV
      Status `/private/var/folders/_6/1yf6sj0950vcg4t91m9ltb5w0000gn/T/jl_AkwWE3/Project.toml`
  [336ed68f] CSV v0.10.8
  [944b1d66] CodecZlib v0.7.0
  [48062228] FilePathsBase v0.9.20
...
      Status `/private/var/folders/_6/1yf6sj0950vcg4t91m9ltb5w0000gn/T/jl_AkwWE3/Manifest.toml`
  [336ed68f] CSV v0.10.8
  [944b1d66] CodecZlib v0.7.0
  [34da2185] Compat v4.5.0
...
Precompiling environment...
  19 dependencies successfully precompiled in 64 seconds
     Testing Running tests...
## Test output starts with no serial precompilation hit
...

by setting ENV["CI"] = "true"

@KristofferC I don't think ENV["CI"] is relevant here. Disabling autoprecompilation is specifically done in Pkg.test until we're ready to do it, and that hasn't changed, just that the Pkg.precompile is now called in the subprocess with the correct setup.

[IanButterworth]

I did however just realise that we used to respect whether the user wanted auto-precompilation here, so I've reinstated that

[KristofferC]

Disabling autoprecompilation is specifically done in Pkg.test until we're ready to do it,

If you look at https://github.com/JuliaLang/JuliaSyntax.jl/actions/runs/3703309958/jobs/6274600432 in the buildpkg step it says:

Precompiling project...
  ✓ JuliaSyntax
  1 dependency successfully precompiled in 3 seconds

That precompile will now be useless. So that's why I am suggesting disabling it on CI so that doesn't run.

[IanButterworth]

Ok, I follow now. Makes sense. Updated. I believe ENV["CI"] can be both "" and "true" on CI, so I tested for that

[DilumAluthge]

I believe ENV["CI"] can be both "" and "true" on CI, so I tested for that

What CI platforms are there that set CI to an empty string?

I think we should only assume that we are running on CI if tryparse(Bool, get(ENV, "CI", "")) === true.

[IanButterworth]

What CI platforms are there that set CI to an empty string?

Only a vague recollection. I've switched it to your suggestion. thanks

[IanButterworth]

On reflection, I'm a bit torn about whether Pkg should be so opinionated about when precompilation happens in CI.

julia-buildpkg isn't just used before julia-runtest, it could be before a script that does something else.

[IanButterworth]

I propose I revert 476b368 (#3281) here and we do julia-actions/julia-buildpkg#26 instead

[staticfloat]

I think what we should do is disable automatic precompilation during Pkg.add() on CI (by having the CI scripts specify that, not something in Pkg) and then have Pkg.test() do a precompile() with the same options as it would use when actually running the tests. If that's what your comment means, then I agree. :)

[IanButterworth]

Almost what I'm saying. There's no standard way to Pkg.add in CI that's like julia-buildpkg julia-runtest so IMO that's up to the user to manage. Both of those steps do Pkg.add internally though.

I'm proposing not precompiling during julia-buildpkg via control in that directly, rather than making Pkg decide.

The rest is accurate

[staticfloat]

In buildkite land I don't think we even manually instantiate (that's what I meant more than Pkg.add(), sorry for imprecise terminology), we just rely on Pkg.test() to do that for us: https://github.com/JuliaCI/julia-test-buildkite-plugin/blob/6859a2c70d88fb5711f423b6bf3e76342b480af4/hooks/command#L28-L30

[IanButterworth]

Cool. Ok, I'm going to go ahead with that plan. I guess we can adapt if needed