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

Initial implementation of "planemo depbash" #310

Closed
wants to merge 68 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@peterjc
Copy link
Contributor

peterjc commented Sep 28, 2015

As described on issue #303, this implements a new planemo depbash command for converting tool_dependencies.xml install recipes into bash scripts.

There is a lot that could be done better, or added, including:

  • refactor to use a visitor pattern instead of my .to_bash() methods
  • expand the command line API with options for paths and filenames
  • complete the action coverage (especially the R/Python/Perl environments)
  • avoid collisions in the download cache which currently assumes unique filenames

Is this suitable to merge as is, or does it need further work?

Does anyone have a better idea for the command name? I have no strong attachment to "depbash" (dependencies to bash script).

I have no objections to squashing down the git commit history here.

P.S. I have considered determining dependencies is out of scope for this work.

peterjc added some commits Sep 21, 2015

Draft implementation of to_bash action methods
TODO:
- Testing
- Escaping/quoting filenames
- Implement missing actions
- Exceptions rather than errors as bash commands?
- Expose as a planemo command
Export environment variables via "source env.sh"
Running "bash install.sh" would not alter environment variables
in the caller's scope.

Using "source install.sh" does alter the caller's environment
variables, but "exit" or any error in strict mode will end the
user's terminal session.

Idea here is "bash install.sh" will download and install the
tool, and also create "env.sh" for use in "source env.sh" within
the Galaxy tool dependency framework.
Expose new command: planemo dep_install
Was going to call this 'planemo shed2bash' or similar, but easier
to run the installation from Python than writing a bash script?
Restore bash conditional indentation.
Also wrap env actions as conditional.
Simple caching of downloaded dependency files.
This is going to be very useful while working on implementing
the rest of 'planemo depbash'.
Simple caching (no attempt to avoid filename collisions)
TODO: Also use an MD5 checksum of the URL here?

peterjc added some commits Sep 30, 2015

Remove setup_virtualenv to_bash() placeholder.
Implementing this will be tricky...
Make DOWNLOAD_CACHE and INSTALL_DIR into absolute paths
This was important as the installation script would often
change directory (especially into a temp working directory)
from where the curl command would fail as the relative path
to the download cache no longer existed.

@peterjc peterjc force-pushed the peterjc:depbash branch from 1c161fe to a4ca795 Oct 1, 2015

@peterjc

This comment has been minimized.

Copy link
Contributor Author

peterjc commented Oct 1, 2015

After some frustration leading to this hack peterjc@7073505 I finally got the basic tests to pass on TravisCI.

Should I do any branch history cleanup, or leave this as it is for any merge?

@peterjc peterjc force-pushed the peterjc:depbash branch from f61338f to 6472190 Oct 7, 2015

@peterjc

This comment has been minimized.

Copy link
Contributor Author

peterjc commented Oct 7, 2015

Minor update to fix the tests since the contents of tests/repos/ changed on the master.

@jmchilton Would you prefer a cleaned-up history? Or just do a --squash merge?

@jmchilton

This comment has been minimized.

Copy link
Member

jmchilton commented Oct 7, 2015

My preference would be this be squashed into a single commit with a nice commit message if you are offering 😄, I don't really enforce this and don't always do it myself and so I have no problem with it being merged as is.

peterjc added a commit that referenced this pull request Oct 7, 2015

Adding new command: planemo dependency_script
This is a squashed commit of pull request #310 for issue #303,
for converting tool_dependencies.xml install recipes into bash
scripts.

There is a lot that could be done better, or added, including:

- refactor to use a visitor pattern instead of my .to_bash() methods
- expand the command line API with options for paths and filenames
- setting defaults like download cache via ~/.planemo.yml
- complete the action coverage (especially the R/Python/Perl environments)
- avoid collisions in the download cache which currently assumes unique filenames

However, this is enough to help with automating dependency
installation in a continuous integration setup like TravisCI.
@peterjc

This comment has been minimized.

Copy link
Contributor Author

peterjc commented Oct 7, 2015

Thanks @jmchilton - done as a squashed merge: f798c7e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment