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

{tools}[dummy/dummy] hub 2.2.2 (REVIEW) #2249

Merged
merged 3 commits into from Jan 17, 2016
Merged

Conversation

nathanhaigh
Copy link
Contributor

Easyconfig for installing hub 2.2.2.

@hpcugentbot
Copy link
Contributor

Automatic reply from Jenkins: Can I test this?

homepage = 'https://hub.github.com/'
description = """hub is a command-line wrapper for git that makes you better at GitHub."""

sources = ['%%(namelower)s-%s-%%(version)s.tgz' % versionsuffix[1:]]
Copy link
Member

Choose a reason for hiding this comment

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

maybe change this to:

source_urls = ['https://github.com/github/hub/releases/download/v%(version)s/']
sources = ['%(namelower)s%(versionsuffix)s-%(version)s.tgz']

@boegel
Copy link
Member

boegel commented Jan 4, 2016

@nathanhaigh: have you tried building it from source? We already have easyconfigs for Go, so may be easy?

@boegel boegel added this to the v2.6.0 milestone Jan 4, 2016
@boegel
Copy link
Member

boegel commented Jan 4, 2016

Jenkins: ok to test

@hpcugentbot
Copy link
Contributor

Easyconfigs unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyconfigs-pr-builder/5397/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

@nathanhaigh
Copy link
Contributor Author

I did try building from source, but hub recommends using the binaries. I haven't any experience with Go and only marginally more with EasyBuild! I wasn't sure what EasyBlock to use and if there are existing tools using it which I could learn from.

Happy to submit a PR for an easyconfig building from source if I can get a bit of guidance.

@hpcugentbot
Copy link
Contributor

Easyconfigs unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyconfigs-pr-builder/5400/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

@boegel
Copy link
Member

boegel commented Jan 5, 2016

@nathanhaigh: ok, no problem, it's just that we try to stay away from binary packages when we can (not sure if it matters a lot for tools written in Go though); let's follow this up in the issue you opened (#2251)

@boegel
Copy link
Member

boegel commented Jan 5, 2016

Test report by @boegel
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in this PR)
Linux SL 6.7, Intel(R) Xeon(R) CPU E5-2670 0 @ 2.60GHz, Python 2.6.6
See https://gist.github.com/15f2a858b0445acb9e55 for a full test report.

@boegel
Copy link
Member

boegel commented Jan 5, 2016

Going in, thanks @nathanhaigh!

@nathanhaigh nathanhaigh changed the title Added easyconfig for hub-2.2.2 {tools}[dummy/dummy] hub 2.2.2 (OK) Jan 5, 2016
@nathanhaigh
Copy link
Contributor Author

Hold off merging this for the moment. I'm experiencing some odd behaviour once I've loaded the module and then run eb to install a tool.

What I've noticed is that during the configuring step of eb blah.eb I get 10's of thousands of hub processes being spawned.

@nathanhaigh nathanhaigh changed the title {tools}[dummy/dummy] hub 2.2.2 (OK) {tools}[dummy/dummy] hub 2.2.2 (WIP) Jan 6, 2016
@nathanhaigh
Copy link
Contributor Author

This is what module show ouputs:

$ module show hub/2.2.2-linux-amd64
-------------------------------------------------------------------------------------------------------------------------
   /mnt/bioinf-7/easybuild/modules/tools/hub/2.2.2-linux-amd64:
-------------------------------------------------------------------------------------------------------------------------
whatis("Description: hub is a command-line wrapper for git that makes you better at GitHub. - Homepage: https://hub.github.co
m/ ")
conflict("hub")
prepend_path("MANPATH","/opt/shared_apps/easybuild/software/hub/2.2.2-linux-amd64/share/man")
prepend_path("PATH","/opt/shared_apps/easybuild/software/hub/2.2.2-linux-amd64/bin")
setenv("EBROOTHUB","/opt/shared_apps/easybuild/software/hub/2.2.2-linux-amd64")
setenv("EBVERSIONHUB","2.2.2")
setenv("EBDEVELHUB","/opt/shared_apps/easybuild/software/hub/2.2.2-linux-amd64/easybuild/hub-2.2.2-linux-amd64-easybuild-deve
l")
prepend_path("PATH","/opt/shared_apps/easybuild/software/hub/2.2.2-linux-amd64")
help([[ hub is a command-line wrapper for git that makes you better at GitHub. - Homepage: https://hub.github.com/

]])

Notice that last prepend_path adds /opt/shared_apps/easybuild/software/hub/2.2.2-linux-amd64 to the PATH. Well, in that directory is an executable called install which is confusing eb during the configuring step. I don't think this should be added to the path.

Other than the hub binary under the bin directory, there is also man pages under share and some auto-completion config files under etc:

$ ls -1 /opt/shared_apps/easybuild/software/hub/2.2.2-linux-amd64
bin
easybuild
etc
install
LICENSE
README.md
share

So I'm wondering how best to deal with all of these things.

@boegel
Copy link
Member

boegel commented Jan 6, 2016

Hmm, for some reason I ended up not hitting the merge button on this on yet, even though I intended to.

The problem you ran into is indeed worrying.

It's caused by the Binary easyblock (from which the PackedBinary used here) also considering '' (a path relative to the install directory) for $PATH. It probably shouldn't be doing that blindly, but conditionally, so we can make sure the install dir is not prepended to $PATH in this case?

@nathanhaigh
Copy link
Contributor Author

The reason this is so bad is that the install script provided by hub actually calls install which it expects on the PATH. Since the install provided by hub is on the PATH, we get recursive calls, ugh!

Not adding the install dir to PATH would work - can you make those changes @boegel?

@nathanhaigh nathanhaigh deleted the hub branch January 10, 2016 22:55
@nathanhaigh nathanhaigh restored the hub branch January 10, 2016 23:31
@nathanhaigh
Copy link
Contributor Author

Damn, deleted my branch by mistake - restored it now :)

@nathanhaigh nathanhaigh reopened this Jan 10, 2016
@hpcugentbot
Copy link
Contributor

Automatic reply from Jenkins: Can I test this?

1 similar comment
@hpcugentbot
Copy link
Contributor

Automatic reply from Jenkins: Can I test this?

This avoids issues with the `installdir` being unconditionally added
to the PATH by the module.
@nathanhaigh
Copy link
Contributor Author

@boegel the latest commit addresses these issues without the need to modify what gets prepended to PATH. Although the latter would be a better solution, at least this easyconfig won't cause these issues now.

@nathanhaigh nathanhaigh changed the title {tools}[dummy/dummy] hub 2.2.2 (WIP) {tools}[dummy/dummy] hub 2.2.2 (REVIEW) Jan 11, 2016
@boegel
Copy link
Member

boegel commented Jan 11, 2016

Jenkins: ok to test


toolchain = {'name': 'dummy', 'version': 'dummy'}

postinstallcmds = ['chmod -x %(installdir)s/install']
Copy link
Member

Choose a reason for hiding this comment

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

hmm, are you sure this is OK? Why is this script there at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, here's more info easybuilders/easybuild-easyblocks#801

The actual install script provided by hub can be seen here: https://github.com/github/hub/blob/master/script/install.sh. Line 22 ends up calling itself recursively because the installdir is added to the PATH.

Making the script non-executable is a workaround.

@hpcugentbot
Copy link
Contributor

Easyconfigs unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyconfigs-pr-builder/5583/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

@boegel
Copy link
Member

boegel commented Jan 17, 2016

Test report by @boegel
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in this PR)
Linux SL 6.7, Intel(R) Xeon(R) CPU E5-2670 0 @ 2.60GHz, Python 2.6.6
See https://gist.github.com/f35de541b5fe44046230 for a full test report.

@boegel
Copy link
Member

boegel commented Jan 17, 2016

lgtm

@boegel
Copy link
Member

boegel commented Jan 17, 2016

Going in, thanks @nathanhaigh!

boegel added a commit that referenced this pull request Jan 17, 2016
@boegel boegel merged commit ef7e722 into easybuilders:develop Jan 17, 2016
@nathanhaigh nathanhaigh deleted the hub branch February 9, 2016 23:21
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.

None yet

3 participants