JuliaPy / Conda.jl

Conda managing Julia binary dependencies
Other
171 stars 57 forks source link

condabin instead of bin #214

Closed mkitti closed 2 years ago

mkitti commented 2 years ago

The conda executable on non-Windows systems is generally in condabin rather than bin. The bin directory is only usually used if that the root conda environment is the active conda environment.

$ which conda
/home/mkitti/anaconda3/condabin/conda
isuruf commented 2 years ago

What's the issue here?

mkitti commented 2 years ago

I'm just noticing this discrepancy:

julia> Conda.conda
"/home/mkitti/anaconda3/bin/conda"

julia> run(`which conda`)
/home/mkitti/anaconda3/condabin/conda

Does it matter?

isuruf commented 2 years ago

No, it doesn't matter.

mkitti commented 2 years ago

Are those two files always the same?

isuruf commented 2 years ago

yes

mkitti commented 2 years ago

What I am thinking is to put this into the activate.sh for the julia-feedstock per https://github.com/conda-forge/julia-feedstock/issues/161

conda_path = $(which conda)
export CONDA_JL_HOME_BACKUP=${CONDA_JL_HOME:-}
export CONDA_JL_HOME = $(conda_path%/*/*}

Is there a better way?

isuruf commented 2 years ago

That's not preferable. That would make the root conda env instead of the julia conda env to be used by Conda.jl. Right now, for CONDA_JL_HOME to work, it needs the conda executable to be in that env as well, but the conda env that julia is installed to might not have the conda executable in it.

mkitti commented 2 years ago

It sounds to me like we need to separate conda from CONDA_JL_HOME/ROOTENV

isuruf commented 2 years ago

Yes

isuruf commented 2 years ago

Or we add micromamba to Yggdrasil and use the micromamba executable.

mkitti commented 2 years ago

For mamba, I'm thinking we might need a Mamba.jl that is separate from this package, perhaps?

mkitti commented 2 years ago

For the current task, I'm trying to figure out how to make Conda.jl work when we are using julia within a conda environment.

mkitti commented 2 years ago

ROOTENV seems to be the correct concept, but it should not be the default environment in all the commands. Rather the default environment should be the current active conda environment indicated by CONDA_PREFIX if it exists, defaulting to ROOTENV if it does not.

ngam commented 2 years ago

but the conda env that julia is installed to might not have the conda executable in it

I thought each conda env will have its own conda and mamba executables as $CONDA_PREFIX/bin/conda and $CONDA_PREFIX/bin/mamba.

I also thought the executables in condabin are simply links of the executables in the parent bin directory (../bin/).

@mkitti why not use $CONDA_PREFIX again here to direct the traffic?

ngam commented 2 years ago

Okay, oh I see what @isuruf is saying (not every env will have conda/mamba exe). As a dirty solution, we can require julia to install the conda/mamba executables with it. Then we can just use $CONDA_PREFIX/bin

mamba install conda mamba

or add them both to the run reqs

mkitti commented 2 years ago

Since we're running this on macOS and Linux only from conda-forge, locating the conda executable seems to be a good strategy to identify the ROOTENV. That can be done in activate.sh.

Next, we need to switch some of the default environments in this package to use the environment identified by $CONDA_PREFIX rather than than the ROOTENV. That is if you say Conda.add( ... ) it should probably add it to the current active environment by default rather than the ROOTENV.

That's a breaking change, so this would probably need to involve a version bump to Conda.jl 2.0.

isuruf commented 2 years ago

That is if you say Conda.add( ... ) it should probably add it to the current active environment by default rather than the ROOTENV.

No, that's not desirable. The default environment should be determined at Conda.jl build time. Changing the default env at runtime is an invitation for disasters.

mkitti commented 2 years ago

That will work as well. We just capture the $CONDA_PREFIX at build time then.

In the case of the 1.7 conda-forge builds, but I think there still needs to be a distinction between ROOTENV and the "active conda environment" at build time.

isuruf commented 2 years ago

Yeah, CONDA_JL_HOME/ROOTENV should be the default environment and something like CONDA_JL_CONDA_EXE should be a build time constant pointing to the conda executable.

ngam commented 2 years ago

I am sorry, I am confused here.

From the julia-feedstock perspective, we only want Conda.jl to basically use the already-installed conda that had already installed julia, right? Why would we care about the CONDA_JL_HOME and ROOTENV variables? The idea is to basically instruct Conda.jl to use some executable we have in CONDA_PREFIX and call it a day. The other variables should be disabled and rendered irrelevant then imo. And I'd very much rather that Conda.jl never interacts with anything outside CONDA_PREFIX --- that is, it should be completely isolated in every and each conda env.

We can produce some sort of a variable in julia, export it, etc. and that's the end of the story. How Conda.jl decides to use said new and distinct variable is outside the scope; and further arrangement is unneeded.

isuruf commented 2 years ago

That's a different issue. We are not talking about julia-feedstock.

ngam commented 2 years ago

That's a different issue. We are not talking about julia-feedstock.

Oh okay... I was referring to this, @mkitti said we were held up by this, hence my confusion: https://github.com/conda-forge/julia-feedstock/issues/161#issuecomment-1013057878

mkitti commented 2 years ago

I was thinking that CONDA_JL_HOME/ROOTENV is where bin/conda lives while CONDA_JL_DEFAULTENV/DEFAULTENV reflects the default environment.

We have functions like prefix that return joinpath(ROOTENV, "envs", sname), so I think the definition of ROOTENV should stay unchanged.

In the julia-feedstock we will do this in activate.sh:

conda_path=$(which conda)
export CONDA_JL_HOME_BACKUP=${CONDA_JL_HOME:-}
export CONDA_JL_HOME = ${conda_path%/*/*}
export CONDA_JL_DEFAULTENV_BACKUP=${CONDA_JL_DEFAULTENV:-}
export CONDA_JL_DEFAULTENV = $CONDA_PREFIX

Does that make sense?

isuruf commented 2 years ago

No, that would be a major breaking change. Also why does bin/conda need an environment? ROOTENV should be the default env and we can have a new variable for the conda executable.

ngam commented 2 years ago

No, that would be a major breaking change. Also why does bin/conda need an environment? ROOTENV should be the default env and we can have a new variable for the conda executable.

@isuruf do you think it is okay to have have different julia envs in different conda envs share the same default conda env through Conda.jl? Does that potentially introduce any issues on the Conda.jl side?

mkitti commented 2 years ago

At the moment, we agree that we need at least two different variables:

  1. One variable to define the location of the conda executable. You proposed CONDA_JL_CONDA_EXE. I have proposed keeping this as CONDA_JL_HOME/ROOTENV.
  2. One variable to define the default environment to manipulate. You proposed we keep using ROOTENV for this I have proposed this as CONDA_JL_DEFAULTENV/DEFAULTENV.

The reason I think we still need ROOTENV to define the location of the conda executable is that we also have code such as prefix. https://github.com/JuliaPy/Conda.jl/blob/f1690184f15c875100e35dde187f3dd2a1235db8/src/Conda.jl#L32-L38 This looks to me that we still need an anchor to a root environment. That is

If I wanted to refer to another a third environment foo, my expectation is that it would be at /home/mkitti/anaconda3/envs/foo. Perhaps there may be a reason to have a nested environment /home/mkitti/anaconda3/envs/julia171/envs/foo. In that case you can just assign both ROOTENV and DEFAULTENV to /home/mkitti/anaconda3/envs/julia171.

We also have code such as _install_conda which is as follows. https://github.com/JuliaPy/Conda.jl/blob/f1690184f15c875100e35dde187f3dd2a1235db8/src/Conda.jl#L191-L216

Earlier, const PREFIX = prefix(ROOTENV). This again looks ROOTENV should be the environment where the conda executable is installed. If it is not, then Conda.jl is going to install Conda in the new default environment. In this case, do we still need two variables?

Perhaps I'm misunderstanding something above.

mkitti commented 2 years ago

No, that would be a major breaking change

I agree that this is breaking which is why I proposed a major version bump to 2.0. The breakage only really occurs if someone defines CONDA_JL_DEFAULTENV. Otherwise, ROOTENV == DEFAULTENV and things are essentially as they were.

isuruf commented 2 years ago

This looks to me that we still need an anchor to a root environment.

Yes, but how is that relevant to where the conda executable is located?

You are under the assumption that the conda executable needs to be in an environment. When we package micromamba which can act as a conda executable, then there's no environment with conda executable. In your scheme how are you going to define a ROOTENV?

mkitti commented 2 years ago

How do we define prefix and PREFIX then? Is anything relative to the conda or mamba executable?

isuruf commented 2 years ago

No. prefix and PREFIX have nothing to do with the conda or mamba executable. They relate to the default conda environment.

mkitti commented 2 years ago

Let me ask this way: where do we put installer.sh in the event that the executable is not found? That's currently based on PREFIX.

https://github.com/JuliaPy/Conda.jl/blob/master/src/Conda.jl#L196

mkitti commented 2 years ago

Let's say I have CONDA_JL_CONDA_EXE=/home/mkitti/miniforge3/bin/conda, but this does not actually exist yet. From that, should I derive that the PREFIX should be /home/mkitti/miniforge3 for the purpose of invoking the installer in _install_conda?

julia> conda |> dirname |> dirname
"/home/mkitti/miniforge3"

or perhaps we should just throw an error?

mkitti commented 2 years ago

I made another attempt at this in https://github.com/JuliaPy/Conda.jl/pull/216 that creates a build time constant based on the environmental variable CONDA_JL_CONDA_EXE.