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.

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

Fix Package Directory Setting #87

Conversation

mjohnson541
Copy link
Contributor

I was trying to build packages dependent on conda-forge Julia and was getting incredibly inconsistent behavior. Running virtually the same build would pass sometimes and not pass other times. Eventually I realized that conda-forge julia (I've verified this for v1.1.1, but I also believe v1.0 should have the same problem) saves packages at ~/.julia not ${PREFIX}/share/julia/site. This causes two problems.

  1. conda build success or failure depends on whether the user has separately installed a package (whether as part of a different conda build, another conda env or completely seperate julia env)
  2. Any Julia packages installed as part of a build aren't saved in the env and have to be installed and built separately after adding to a conda env

I propose adding this segment to the build script. This sets the JULIA_DEPOT_PATH to be ${PREFIX}/share/julia/site whenever the conda env is activated (and unsets when deactivated) which causes julia to install packages inside the env and fixes the above issues. I've successfully built a couple dependent packages on osx with this fix.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

recipe/build.sh Outdated
ACTIVATE_ENV="${PREFIX}/etc/conda/activate.d/env_vars.sh"
DEACTIVATE_ENV="${PREFIX}/etc/conda/deactivate.d/env_vars.sh"
if [ -f "$ACTIVATE_ENV" ]; then
echo "export JULIA_DEPOT_PATH=\"${PREFIX}/share/julia/site:\$JULIA_DEPOT_PATH\"" >> "$ACTIVATE_ENV"
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I've tried to adopt the cleaner format used there.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you're going to create an activate.sh / deactivate.sh, then I think you may want to include something like this in build.sh:

https://github.com/conda-forge/openjdk-feedstock/blob/57916550977ace39ff53885556d8ee5a30bf254a/recipe/build.sh#L181-L187

@mjohnson541
Copy link
Contributor Author

I know very little about the compilers involved and conda forge, so I have no idea why this is occurring and I don't know how to debug the error on the osx build, but hopefully this is useful to someone later.

@mjohnson541
Copy link
Contributor Author

It looks like with #115 merged this PR passes tests. What else needs done to merge this?

@mkitti
Copy link
Contributor

mkitti commented Aug 19, 2021

It's not clear to me that moving the JULIA_DEPOT_PATH alone would necessarily maximize reproducibility. While it may help in the near term, it would not necessarily allow you to reproduce the situation in the long term as new Julia packages are introduced.

Besides the JULIA_DEPOT_PATH, perhaps we should also activate a corresponding Julia environment:
https://pkgdocs.julialang.org/v1.2/environments/

This can be done by setting the JULIA_PROJECT environmental variable:
https://docs.julialang.org/en/v1/manual/environment-variables/#JULIA_PROJECT

The idea is that for each conda environment, there should be a corresponding distinct Julia environment.

@mjohnson541
Copy link
Contributor Author

Sorry, I'm a bit confused.

As far as I'm aware the introduction of new Julia packages has nothing to do with the specific definition of JULIA_DEPOT_PATH, julia updates the package registry files (I believe every time you try to update/install anything) there so any new packages will show up there as they enter the registry. Moving JULIA_DEPOT_PATH inside the environment doesn't lock the registry.

I'm also not sure what the point would be of both moving the depot inside the environment and specifying the environment given the base julia environment would be specific to the conda environment. But maybe you're proposing that instead of moving the depot path we should assign conda-forge a specific environment within julia to activate when launching julia? It feels a bit dangerous to me, but I think that design would probably be good enough to make installs built on conda-forge julia consistent and it would probably save memory for people with multiple julia installs as well.

I believe the solution I have here should be sufficient although the julia environments approach would also be a valid solution to this problem.

@mkitti
Copy link
Contributor

mkitti commented Aug 20, 2021

It is likely me who is confused since I am new to Conda's internals.

My understanding here is that when you do conda activate juliatempenv that it would be equivalent to the following using the version in recipe/scripts/activate.sh

(juliatempenv) $ echo $CONDA_PREFIX
/home/mkitti/anaconda3/envs/juliatempenv
(juliatempenv) $ echo $JULIA_DEPOT_PATH

(juliatempenv) $ export JULIA_DEPOT_PATH="$CONDA_PREFIX/share/julia/site:$JULIA_DEPOT_PATH" 
(juliatempenv) $ echo $JULIA_DEPOT_PATH
/home/mkitti/anaconda3/envs/juliatempenv/share/julia/site:
(juliatempenv) $ julia -e "println(DEPOT_PATH)"
["/home/mkitti/anaconda3/envs/juliatempenv/share/julia/site", "/home/mkitti/.julia", "/home/mkitti/anaconda3/envs/juliatempenv/local/share/julia", "/home/mkitti/anaconda3/envs/juliatempenv/share/julia"]

(juliatempenv) $ julia -e "using Pkg; Pkg.status()"
      Status `~/.julia/environments/v1.6/Project.toml`
...

Because my JULIA_DEPOT_PATH was empty before, we now have a trailing :. This has the effect of prepending the specified depot path to the default one, which still includes ~/.julia. Since julia depots stack, the default environment still refers to the default v#.# (e.g. v1.6) environment in ~/.julia as opposed to a new one in ~/$CONDA_PREFIX/share/julia/site.

To make the environment one in ~/$CONDA_PREFIX/share/julia/site, we would need to set JULIA_PROJECT like this:

export JULIA_PROJECT="$CONDA_PREFIX/share/julia/site/environments/$CONDA_DEFAULT_ENV"
(juliatempenv) $ julia -e "using Pkg; Pkg.status()"
      Status `~/anaconda3/envs/juliatempenv/share/julia/site/environments/juliatempenv/Project.toml` (empty project)

In Julia 1.7, this "shared" environment in the depot could be referred to as @juliatempenv due to JuliaLang/julia#40025 .

The alternative might be to not use a trailing : if JULIA_DEPOT_PATH is previously empty, in which the depots will not stack. I note that you do not have a trailing : in the version in build.sh, so perhaps that is intentional.

Copy link
Contributor

@mkitti mkitti left a comment

Choose a reason for hiding this comment

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

After discussing this with @stuarteberg, I realized that the activate.sh and deactivate.sh scripts in recipe/scripts/ were not being used.

My suggestions copy code from https://github.com/conda-forge/openjdk-feedstock/blob/57916550977ace39ff53885556d8ee5a30bf254a/recipe/build.sh#L181-L187 into build.sh which will copy the recipe/scripts/*.sh into ${PREFIX}/etc/conda/activate.d or ${PREFIX}/etc/conda/deactivate.d.

Next, we then need to choose whether to stack the Julia depots or not. Stacking has the advantage of not replicating things already contained in ~/.julia or system depots, and also creates a common Julia parent environment. However, this also means that we need to create a unique environment that lives within $CONDA_PREFIX. The other implication is that packages added to the default environment in ~/.julia/environments/v#.# will be inherited. That could be seen as an advantage or disadvantage.

An alternative solution would be to not stack the Julia depots which would completely isolate the Julia environments in each conda environment.

recipe/build.sh Outdated Show resolved Hide resolved
recipe/scripts/activate.sh Outdated Show resolved Hide resolved
recipe/scripts/deactivate.sh Outdated Show resolved Hide resolved
@mkitti mkitti mentioned this pull request Sep 2, 2021
@mkitti
Copy link
Contributor

mkitti commented Sep 9, 2021

Now that Julia 1.6.2 is available via conda-forge, my attention with regard to this repository is with with this pull request. @mjohnson541, are you still interested in working on this or would you prefer me to fork to push this through?

@mjohnson541
Copy link
Contributor Author

Shoot, sorry, things have been pretty hectic. This looks fine to me, although I don't really have time to set stuff up and test it right now. Feel free to fork and push it though.

* 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#.#.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants