Skip to content
This repository has been archived by the owner on Jan 30, 2019. It is now read-only.

Add support for activate/deactivate scripts #85

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

campos-ddc
Copy link
Contributor

Scripts listed under 'activate' and 'deactivate' contexts in environment.yml will be linked into $PREFIX/etc/conda/(de)?activate.d, where they will be executed by 'source (de)?activate'

Relates to #83

Example:

name: oracle
dependencies:
  - oracle_instantclient

# Note that relative paths are relative to this file's location
activate:
  - set_oraclehome.sh
deactivate:
  - unset_oraclehome.sh 

Scripts listed under 'activate' and 'deactivate' contexts in
environment.yml will be linked into $PREFIX/etc/conda/(de)?activate.d,
where they will be executed by 'source (de)?activate'
@tacaswell
Copy link

👍

This is independent of the activate/deactivate scripts that are install per-package correct?

@campos-ddc
Copy link
Contributor Author

Yes, unless you try to install a script with the same name as one obtained
from a package.

Note that this problem already exists if two packages have files with the
same name.
On Jun 3, 2015 11:39, "Thomas A Caswell" notifications@github.com wrote:

[image: 👍]

This is independent of the activate/deactivate scripts that are install
per-package correct?


Reply to this email directly or view it on GitHub
#85 (comment).

@campos-ddc
Copy link
Contributor Author

Oops, I actually rename files to 0.sh, 1.sh etc..

So there would only be an issue if a package contains a file with those names.

@tacaswell
Copy link

#66 is vaguely relevant to this and stalled by my lack of windows knowledge.

I am not sure renaming is a good idea (even prefixing with integers) as it can clobber user specified order (sans some special casing in the ordering).

@campos-ddc
Copy link
Contributor Author

Makes sense.

I used numbers to make sure that they will be executed in the order they are described in environment.yml, as far as I know, find in the activate script will return filenames sorted alphabetically.

How are you sorting scripts in your pull request? Alphabetically? Modification date?

@tacaswell
Copy link

alphabetically. I think they are currently coming back in random order,
but I am no longer sure why I think that (I don't remember if that PR was
due to something actually going wrong or just general paranoia that the
system can return the list in any order).

On Wed, Jun 3, 2015 at 11:15 AM Diogo Campos notifications@github.com
wrote:

Makes sense.

I used numbers to make sure that they will be executed in the order they
are described in environment.yml, as far as I know, find in the activate
script will return filenames sorted alphabetically.

How are you sorting scripts in your pull request? Alphabetically?
Modification date?


Reply to this email directly or view it on GitHub
#85 (comment).

@campos-ddc
Copy link
Contributor Author

Alphabetically would work perfectly with my changes, there shouldn't be any conflict.

I commented in #66 , it looks like Windows is already sorting alphabetically (but it only lists files at the current dir, no recursion as we have in unix!).

Also, your PR is still valid, find in fact does not return files in any order.

@tswicegood
Copy link
Contributor

This one gives me pause. I like the idea, but something we have to consider here is the complexity that it adds. To this point, an environment spec is nothing more than one file. If we add this, it's 1+n file(s).

To add this in its current state, we would need add error handling to main_create and main_update to ensure that an environment wasn't created unless it could be completely instantiated. I'm not sure if that's a good or bad idea.

Another option that keeps this as a single file is to allow inlining of these activation scripts. Something like this:

- activate:
  - posix: |
      #!/bin/bash
      echo "I'm in a *nix system"
  - win: |
      echo "This is a bat file in Windows"

Thoughts?

@campos-ddc
Copy link
Contributor Author

IMO, a few points to consider:

  • The complexity only exists if the user wants to have activate/deactivate scripts.
  • Using activate/deactivate files (instead of text) fits perfectly with the way that pre-built packages deploy their activation scripts.
  • main_create and main_update already suffer from this problem (they do not check that activation scripts installed by packages can be completely instantiated). If this is a behavior we want, it should be implemented regardless of adding activation scripts to environment.yml.
  • Real script files can be executed independently, without going through conda. This can be useful for testing them.

I like the idea of having everything in a single file, but writing bash/bat scripts as text in a .yml file seems a bit odd. I guess it would work just fine for simple examples, but if I had a 200 line script, I wouldn't want it in text within another file (assuming we'll ever have a 200 line script being needed).

@tswicegood
Copy link
Contributor

I'm not sold on the need of one file, I just realize that it is a change we need to think about. For example, @malev just added the ability to upload and download an environment to Anaconda Server. With this change, now we have to adjust that to make sure that correct files are being sent to the correct places.

To address a couple of points:

The complexity only exists if the user wants to have activate/deactivate scripts.

This is true to the user, but not us as maintainers of the package. We have to be able to manage the complexity on our end as well and while it seems rather straight forward, there are moving parts (like dealing with "remote" packages that have been uploaded) that makes moving from a single file to a single file plus N other files unappealing.

main_create and main_update already suffer from this problem (they do not check that activation scripts installed by packages can be completely instantiated). If this is a behavior we want, it should be implemented regardless of adding activation scripts to environment.yml.

These are not quite the same. An environment.yml installs packages that may or may not actually do what they're supposed to. That's left as an exercise to the package maintainer and pushes the complexity of verification further down the stack in order to keep conda-env simple.

That's not to say that there's a place for this. I think there is, I think we just need to figure out the best way to tackle the problem.


To bring the conversation back to the main goal: the script you used as an example I assume is setting environment variables. Is that correct? If so, what about adding a section that specifies the variables. Something like:

- env:
  - ORACLE_HOME: "{CONDA_ENV_PATH}/opt/oracle"

Then instead of copying files over this would create a _conda-environment-variables.sh file or some such that sets all of these as environment variables. Thoughts? Is there a broader case where the file makes more sense?

@campos-ddc
Copy link
Contributor Author

Yes, I'm implementing another pull request right now that does just that (side note, I also had to add jinja2 support to reference relative paths):

name: app

dependencies:
  - cogapp==2.4
  - colorama==0.3.3

environment:
  ORACLE_HOME: "$CONDA_ENV_PATH/opt/oracle"
  PYTHONPATH:
    - {{ root }}/source/python

For me, it will make the activate and deactivate scripts unnecessary.

That being said, having full control of a bash/bat script is more flexible than just setting environment variables, so I thought that the scripts could still be useful.

I had not considered recent changes in conda env upload/download.

Would you like to wait until I finish and open the other pull request? We can continue this discussion/abort this pull request if the other one is better.

@tswicegood
Copy link
Contributor

That sounds like a solid plan. Thanks for tackling this @campos-ddc.

@campos-ddc
Copy link
Contributor Author

Just opened #88 , take a look there.

@tacaswell
Copy link

I think there is a case for allowing full scripts. I have a compelling use-case where I would like to have an environment starts several server processes to build a test stand environment (but just setting the ENVs is also great!).

@campos-ddc
Copy link
Contributor Author

I had a look at conda env upload, and could quite figure out why it necessary...

I thought that environemnt.yml should always be a part of your repository, along with thew source code. Why would it be uploaded as a standalone file in binstar?

@malev , what use case did you have when implementing this?

@malev
Copy link
Contributor

malev commented Jun 8, 2015

@campos-ddc not every conda user has a repository. Many conda users are focused on jupyter notebooks. The idea behind conda env upload is to share environments between those users.

@tadeu
Copy link

tadeu commented Sep 8, 2015

@malev These changes would be very useful, is there something else you need in order to merge, besides resolving the conflicts?

@vatsan
Copy link

vatsan commented Sep 20, 2016

Is this going to be merged?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants