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

add support for preinstallopts and install in subdirectory to Tarball generic easyblock #1989

Merged
merged 6 commits into from
Apr 17, 2020

Conversation

lexming
Copy link
Contributor

@lexming lexming commented Mar 12, 2020

Update for tarball easyblock to make it (1) more flexible and (2) usable as a component in a bundle:

  • Currently, tarball deletes and re-creates installdir on each installation, removing any installation from previous components. This PR adds an extra variable final_dir that is similar to start_dir and can be used to target sub-directories of installdir, preserving installations from previous components. The default behavior of tarball will be unaffected.
  • Adds support for using preinstallopts with tarball. Any commands given through preinstallopts will be executed before the copy of files to install directory.

@lexming
Copy link
Contributor Author

lexming commented Apr 8, 2020

I tested the execution of the new preinstall_cmd and preinstallopts in the following cases and it successfully works in all of them:

  • Single preinstallopts
'preinstallopts' = 'touch A && touch B && '
  • Single preinstall_cmd
'preinstall_cmd' = 'touch C && touch D'
  • Both preinstallopts and preinstall_cmd
'preinstallopts' = 'touch A && touch B && '
'preinstall_cmd' = 'touch C && touch D'

@ocaisa
Copy link
Member

ocaisa commented Apr 8, 2020

@lexming Coincidentally PR easybuilders/easybuild-framework#3270 was opened today in framework that intends to cover (some of) the same ground as this, can you check if that approach would suit your needs?

@lexming
Copy link
Contributor Author

lexming commented Apr 8, 2020

@ocaisa that PR merges the folders instead of installing them in different subfolders. That will not work for bundles, at least not in a general sense, as the source code of different projects will probably have collisions.

@lexming lexming changed the title tarball: add support for preinstallopts and final_dir tarball: add support for preinstallopts and install in subdirectory Apr 8, 2020
@lexming
Copy link
Contributor Author

lexming commented Apr 8, 2020

@ocaisa I think that both options can complement each other and could be added to tarball. Maybe a variable called install_type that would accept merge or subdir as options. I'll wait to see what @Darkless012 proposes.

@Darkless012
Copy link
Contributor

Darkless012 commented Apr 9, 2020

I might be repeating, but let me summarize what is needed by this change:

  1. Don't remove install dir in tarball - on which we all agree, because this should be done by easyconfig and brings trouble to Bundles.
  2. Allow changing (in some way) installdir so that conflicts will not arise.

Do you have any specific use-case other than using this in Bundle?

I needed step 1 Because I had MakeCP and Tarball. MakeCP would create installdir/bin/executable and then Tarball should copy its content (into installdir which includes bin folder) as well. Which is my need in merging folders instead of remove/copy. So there should be extra param "merge folder" which should not be default as you mentioned.
I can reverse the order of components for sure. But Tarball still shouldn't remove folder.
But as you pointed with install_in_subdir I might just have different folder with bin/executable and add it to the PATH?

I didn't thought about having multiple Tarballs in bundle, which is valid point. I like the idea of having install_in_subdir.
Just want point out that if you are already installing it into bundle, you might want to specify exact path within the installdir? Which is basically what I'm doing with MakeCp (files_to_copy).
Sure you can do mv/ln -s in postinstallcmds, where the install_in_subdir makes sence, because you will have always same name structure to copy from?

Does install_in_subdir has any other use besides Bundle?
"merge_folder" probably doesn't have since installdir should be wiped in solo Tarball installation.

@lexming
Copy link
Contributor Author

lexming commented Apr 12, 2020

List of use cases and required actions needed for each of them:

  1. Installation of a single tarball

    • Wipe existing installation directory (we do not want any unexpected behaviour caused by leftovers)
    • Unpack and copy tarball
  2. Installation of a multiple tarballs, where one is the main package and the other are addons that have to be decompressed in paths below the main one

  3. Installation of multiple tarballs, where all of them have to be merged in a common installation directory. Useful for Bundles were tarball easyblock comes after other easyblocks that have already populated the installation directory.

    • Installation directory will be already wiped and populated by previous easyblock
    • Copy files to installation directory without wipe and enabling dirs_exist_ok
  4. Installation of multiple tarballs, where each of them is independent from the others and live in their own subdirectory. Useful for Bundles with multiple tarball easyblocks.

    • Wipe installation sub-directory
    • Unpack and copy tarball to specific sub-directory

Cases 1 and 2 are already supported. The default behaviour of tarball should not change to avoid breaking existing easyconfigs.
Support for case 3 and 4 will be added by this PR. Since these two options are exclusive, I propose to have a single option that can be set to one or the other.

@lexming
Copy link
Contributor Author

lexming commented Apr 12, 2020

@Darkless012 @ocaisa I have added the option install_type to handle default installations and also the options of merging the contents of the tarball or extracting them in their own sub-directory. Please test it and let me know your thoughts.

I've tested it combining several tarball components in a single bundle with various combinations of options (None, merge, subdir) and all worked well on my end.
With this PR all four use cases listed in #1989 (comment) should be covered.

@ocaisa
Copy link
Member

ocaisa commented Apr 17, 2020

@lexming PR looks good, can you share that test case (here or in a gist) just so I repeat the tests for myself?

@lexming
Copy link
Contributor Author

lexming commented Apr 17, 2020

@ocaisa I put the dummy easyconfig to test this PR in https://gist.github.com/lexming/3477ca1390b1965affb6338efc5f1dd7
It's just a stripped down version of my PR for Alpha (easybuilders/easybuild-easyconfigs#9994) with the updated settings from this PR for tarball.
The version I'm sharing uses subdir and should fulfil the sanity checks. The python package has its own lib folder and pyinclude and alpha are in their own folder. You can change the settings for install_type to merge or None and check the different results.

Copy link
Member

@ocaisa ocaisa left a comment

Choose a reason for hiding this comment

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

@ocaisa ocaisa merged commit 84c560a into easybuilders:develop Apr 17, 2020
@lexming lexming deleted the tarball branch April 17, 2020 13:53
@boegel boegel changed the title tarball: add support for preinstallopts and install in subdirectory add support for preinstallopts and install in subdirectory to Tarball generic easyblock Apr 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants