Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Base.runtests: set a temporary Pkg depot path #42358

Merged
merged 2 commits into from
Sep 23, 2021
Merged

Base.runtests: set a temporary Pkg depot path #42358

merged 2 commits into from
Sep 23, 2021

Conversation

DilumAluthge
Copy link
Member

No description provided.

@DilumAluthge DilumAluthge added the testsystem The unit testing framework and Test stdlib label Sep 23, 2021
@DilumAluthge DilumAluthge added the test This change adds or pertains to unit tests label Sep 23, 2021
@KristofferC
Copy link
Sponsor Member

Would be good to say something in the commit message why this is needed now. Something with Buildkite?

@DilumAluthge
Copy link
Member Author

DilumAluthge commented Sep 23, 2021 •
edited
Loading

Buildbot, actually. This fixes one source of intermittent Buildbot failures.

More context on Slack: https://julialang.slack.com/archives/C01F9PX2UMU/p1632263882053700

Ahhh, I know why this is happening. The “fake” JLLs don’t get compiled into the system image, because we don’t want to run their __init__() until they’re imported.

This means that when we try to load the module, we compile, and we save out the .ji file. And when we’re running Base.runtests() we don’t have a default depot set, I bet.

vtjnash
vtjnash approved these changes Sep 23, 2021
View reviewed changes
Copy link
Sponsor Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we also need to set JULIA_PROJECT?

@DilumAluthge
Copy link
Member Author

I think we also need to set JULIA_PROJECT?

Hmmm, I don't think we need to. If JULIA_DEPOT_PATH is provided, and no project is provided, then Julia should just use the default global project in our temp depot.

vtjnash
vtjnash reviewed Sep 23, 2021
View reviewed changes
Copy link
Sponsor Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does buildbot / buildkite use this code? I thought they generally launched the runtests.jl file directly (and avoided the ncores /= 2here).

@DilumAluthge
Copy link
Member Author

I'm not sure about Buildbot - @staticfloat will know.

Buildkite calls Base.runtests. However, we explicitly set the ncores keyword argument.

@staticfloat
Copy link
Sponsor Member

Yes, buildbot uses Base.runtests() as well: https://github.com/JuliaCI/julia-buildbot/blob/63603639f20d848cf2173b47aee31ec8f9a427b2/master/separated_testing.py#L16

@DilumAluthge DilumAluthge added the status:merge me PR is reviewed. Merge when all tests are passing label Sep 23, 2021
@DilumAluthge DilumAluthge merged commit 2c9e051 into master Sep 23, 2021
@DilumAluthge DilumAluthge deleted the dpa/runtests-temp-depot branch September 23, 2021 21:35
@DilumAluthge DilumAluthge removed the status:merge me PR is reviewed. Merge when all tests are passing label Sep 23, 2021
KristofferC pushed a commit that referenced this pull request Sep 28, 2021
@DilumAluthge @KristofferC
Base.runtests: set a temporary Pkg depot path (#42358)
ec95d9a
@sostock
Copy link
Contributor

sostock commented Oct 15, 2021 •
edited
Loading

I think we also need to set JULIA_PROJECT?

Hmmm, I don't think we need to. If JULIA_DEPOT_PATH is provided, and no project is provided, then Julia should just use the default global project in our temp depot.

Does it? At least for me, this PR breaks the revise option of Base.runtests:

(@v1.8) pkg> st
Status `~/.julia/environments/v1.8/Project.toml`
  [6e4b80f9] BenchmarkTools v1.2.0
  [33e6dc65] MKL v0.4.2
  [295af30f] Revise v3.1.20

julia> Base.runtests("ranges", revise=true)
ERROR: LoadError: ArgumentError: Package Revise not found in current path.
- Run `import Pkg; Pkg.add("Revise")` to install the Revise package.
Stacktrace:
 [1] require(into::Module, mod::Symbol)
   @ Base ./loading.jl:989
 [2] top-level scope
   @ ~/Development/julia/usr/share/julia/test/runtests.jl:26
in expression starting at /home/sebastian/Development/julia/usr/share/julia/test/runtests.jl:25

@sostock sostock mentioned this pull request Oct 18, 2021
Closed
@mkitti mkitti mentioned this pull request Dec 27, 2021
Closed
mkitti added a commit to ngam/julia-feedstock that referenced this pull request Dec 27, 2021
@mkitti
Modify patch to match upstream JuliaLang/julia#42358. Use addenv.
480090b
@DilumAluthge DilumAluthge added backport 1.6 Change should be backported to release-1.6 backport 1.7 labels Dec 27, 2021
@mkitti mkitti mentioned this pull request Dec 28, 2021
Closed
@DilumAluthge DilumAluthge mentioned this pull request Dec 28, 2021
Merged
ngam added a commit to conda-forge/julia-feedstock that referenced this pull request Dec 29, 2021
@ngam @mjohnson541 @mkitti
reorganize & cleanup (#157)
bd5133e 
* set JULIA_DEPOT_PATH when conda env activated

* simplify recipe and add back up variables

* don't run juliarc.jl

* improve activate.d/deactivate.d file editing

* Apply suggestions from code review by @mkitti

* This uses the activate.sh and deactivate.sh scripts that backup the environment variable.
* Create a new shared environment located in the site depot for the environment rather than continuing to use the default one in ~/.julia/environments/v#.#.

* reorganize

* increase bld no

* try building osx with system mbedtls

* only apply three new patches to linux for now

* reorganize order of patches

* Revert "try building osx with system mbedtls"

This reverts commit 1b1d170.

* comment out juliarc.jl

* Update build.sh

* try runtests math for osx

* test "opaque_closure", "filesystem"

* go all in on testing

* revert previous; parallelize + add "math" to list

- use all processors: ncores=ceil(Int, Sys.CPU_THREADS
- move math up to regular tests (no longer necessary to stop it from running on osx)

* Update build.sh

* Update activate.sh

* Revert "Update activate.sh"

This reverts commit fe2b9c9.

* new patches

* correct patches in meta

* change of patches

* Update meta.yaml

* Modify patch to match upstream JuliaLang/julia#42358. Use addenv.

* Re-add JULIA_PROJECT to be "@$CONDA_DEFAULT_ENV"

* Revert "Re-add JULIA_PROJECT to be "@$CONDA_DEFAULT_ENV""

This reverts commit 1e312a1.

* Set JULIA_PROJECT and JULIA_LOAD_PATH according to conda environment

* Test loading and cmdlineargs with default JULIA_PROJECT

* Extract env name as last directory of CONDA_PREFIX

* Remove JULIA_LOAD_PATH while testing loading

* Update build.sh

* Delete recipe/old_unused directory

* reorganize patches and delete unnecessary files

* remove DS_STORE

* oops place back patches

* Apply suggestions from code review, remove "unique"

Co-authored-by: ngam <[email protected]>

Co-authored-by: Matt Johnson <[email protected]>
Co-authored-by: Mark Kittisopikul <[email protected]>
Co-authored-by: Mark Kittisopikul <[email protected]>
KristofferC pushed a commit that referenced this pull request Jan 5, 2022
@DilumAluthge @KristofferC
Base.runtests: set a temporary Pkg depot path (#42358)
e13f8bf 
(cherry picked from commit 2c9e051)
@KristofferC KristofferC mentioned this pull request Jan 10, 2022
Merged
50 tasks
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
@DilumAluthge @LilithHafner
Base.runtests: set a temporary Pkg depot path (JuliaLang#42358)
960539a
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
@DilumAluthge @LilithHafner
Base.runtests: set a temporary Pkg depot path (JuliaLang#42358)
af6686f
KristofferC pushed a commit that referenced this pull request Mar 15, 2022
@DilumAluthge
Base.runtests: set a temporary Pkg depot path (#42358)
017540c 
(cherry picked from commit 2c9e051)
KristofferC pushed a commit that referenced this pull request Mar 16, 2022
@DilumAluthge
Base.runtests: set a temporary Pkg depot path (#42358)
74d6f07 
(cherry picked from commit 2c9e051)
@KristofferC KristofferC removed the backport 1.6 Change should be backported to release-1.6 label May 16, 2022
staticfloat pushed a commit that referenced this pull request Dec 23, 2022
@DilumAluthge @staticfloat
Base.runtests: set a temporary Pkg depot path (#42358)
01ff0b5 
(cherry picked from commit 2c9e051)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vtjnash vtjnash vtjnash approved these changes

@staticfloat staticfloat Awaiting requested review from staticfloat

Assignees
No one assigned
Labels
test This change adds or pertains to unit tests testsystem The unit testing framework and Test stdlib
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@DilumAluthge @KristofferC @staticfloat @sostock @vtjnash