fix Lmod spider output in generated modules#1583

pescobar · 2016-01-29T10:33:17Z

No description provided.

hpcugentbot · 2016-01-29T10:43:09Z

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2606/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

hpcugentbot · 2016-01-29T11:00:07Z

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2607/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

hpcugentbot · 2016-01-29T11:31:44Z

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2608/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

wpoely86 · 2016-02-08T09:30:02Z

easybuild/tools/module_generator.py

            'version': self.app.version,
            'description': description,
-            'whatis_lines': '\n'.join(["module-whatis {%s}" % line for line in whatis]),
+            'whatis_lines': '\n'.join(["module-whatis { Description: %s}" % line for line in whatis]),


why the whitespace before Description?

wpoely86 · 2016-02-08T09:30:50Z

Great catch @pescobar !

lgtm

wpoely86 · 2016-02-08T09:30:57Z

retest this please

pescobar · 2016-02-08T09:48:13Z

@wpoely86 I was discussing this with @geimer by email (he changed this code) and this fix is not the right one

whatis section now can have different lines like this:

whatis = [
    'MVAPICH2',
    'Version: 2.1',
    'Category: library, runtime support',
    'Keywords: System, Library',
    'Description: MPI-2 implementation for Infiniband',
    'URL: http://mvapich.cse.ohio-state.edu/overview/mvapich2/',
]

with this patch if you add multiple whatis lines each of them would have a Description: string in front of it.

We need to look for a better solution. I am really interested in getting proper spider output for next easybuild release

hpcugentbot · 2016-02-08T09:48:29Z

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2643/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

wpoely86 · 2016-02-08T09:53:20Z

@pescobar yeah, I was just looking into the source of spider to see how it works. We should definitely chance this. Do you have time to do this properly?

pescobar · 2016-02-08T09:58:01Z

we must fist agree about which is a proper solution. It's not clear to me. I also asked @boegel but I think he doesn't have a clear opinion about this neither.

wpoely86 · 2016-02-08T10:38:59Z

Okay, we should generate a whatis section like:

module-whatis "Name: Globus-4.0"
module-whatis "Version: CTSSV 4"
module-whatis "Category: grid"
module-whatis "URL: http://software.teragrid.org/install.htm"
module-whatis "Description: Defines the TeraGrid Globus 4.0 User Environment"

We already know the name, version, url and description. For category we can use the moduleclass (maybe optional?)
The easyconfigs already have keyword whatis that works like:

whatis = ["Url: foo", "Description: bar"]

We should merge the contents of it with the default generate one (meaning, append&overwrite).

geimer · 2016-02-08T11:42:56Z

@wpoely86: We should give users the freedom to customize the contents of whatis to whatever they want. Different sites may have different policies w.r.t. what whatis should contain. For example, on our previous systems module whatis only showed a short one-line description of the package, while module help (which is currently filled with the contents of the description variable) showed detailed usage instructions, where to find additional information, the local contact, etc. Also, remember that Tmod only shows the last entry when executing module whatis, in which case the Description: prefix is redundant. By merging the user-provided contents with some automatically generated stuff, you prohibit customization and enforce a specific policy to everyone using EB. Moreover, how should a merge algorithm look like? What if a site only wants the one-liner specified in the easyconfig rather than the full description? How do you want to handle inconsistencies due to customization, e.g., if a site wants the URL to point to local documentation rather than the homepage? Handle the strings as key-value pairs? This seems to open a big can of worms...

To me, the culprit is that Lmod relies on a specific formatting of whatis -- following the TACC site policy. Thus I see this more as an Lmod issue than an EB issue. My current thinking is that the behavior of module spider should be configurable in Lmod (e.g., use the current behavior, print all whatis strings, extract the first paragraph of module help, ...).

@rtmclay: Any thoughts on this?

wpoely86 · 2016-02-08T12:59:17Z

@geimer First, does module whatis work for you? I never used it and I get tcl errors on EB modules.

What you suggest it as site customizing of everything? I don't think we can do that? I think we should do the following:

Add all the whatis line we can based on the information in the easyconfig (description, homepage, ...)
If a whatis line is present in the easyconfig, this overwrites the defaults. (so a site can add customizations per easyconfig if they wish, they can even use --amend).
We support a generic header and footer in which additional customization can be done.

And it Tmod has issues with multiple lines, we can make sure that description is always the last one. Or just add a config option to merge all whatis fields or not.

However, that the users gets to see when the run whatis, help or spider is up to Lmod. If you want to customize, use hooks. EB cannot do anything there.

boegel · 2016-02-08T15:14:57Z

I agree with @wpoely86 that we can probably do a better job by default. But if a whatis is specified in the easyconfig, that should indeed win, otherwise sites lose the ability to customise things (and we really need to keep the whatis stuff backward compatible now).

@pescobar: I think what you're after is just adding Description: to the line that defines description =, higher up from where you're patching things now

wpoely86 · 2016-02-08T16:05:28Z

@rtmclay, it is normal that spider doesn't show all whatis fields?

$ ml show gsissh/4.1
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
   /home/ward/tmp/Lmod/tcl/tacc/apps/gsissh/4.1:
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
whatis("Name: Gsissh ")
whatis("Version: 4.1 ")
whatis("Category: grid ")
whatis("URL: http://www.globus.org/toolkit ")
whatis("Description: Globus GSI OpenSSH utility ")
prepend_path("PATH","/opt/gsi-openssh-4.1/bin")
prepend_path("MANPATH","/opt/gsi-openssh-4.1/man")
prepend_path("LIBPATH","/opt/gsi-openssh-4.1/lib")
prepend_path("LD_LIBRARY_PATH","/opt/gsi-openssh-4.1/lib")
prepend_path("SHLIB_PATH","/opt/gsi-openssh-4.1/lib")
setenv("GLOBUS_LOCATION","/opt/gsi-openssh-4.1")
setenv("GLOBUS_PATH","/opt/gsi-openssh-4.1")
setenv("SASL_PATH","/opt/gsi-openssh-4.1/sasl")
setenv("RSHCOMMAND","/opt/gsi-openssh-4.1/bin/gsissh")
setenv("MYPROXY_SERVER","myproxy.teragrid.org")
setenv("GLOBUS_TCP_PORT_RANGE","50000,51000")
help([[The gsi-openssh-4.1 modulefile defines the default paths and environment
variables needed to use the Globus software and utilities
available in /share/gsi-openssh-4.1, placing them before the vendor-supplied
paths in PATH and MANPATH.:
]])

And

$ ml spider gsissh/4.1

------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  gsissh: gsissh/4.1
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
    Description:
      Globus GSI OpenSSH utility 

    This module can be loaded directly: module load gsissh/4.1

    Help:
      The gsi-openssh-4.1 modulefile defines the default paths and environment
      variables needed to use the Globus software and utilities
      available in /share/gsi-openssh-4.1, placing them before the vendor-supplied
      paths in PATH and MANPATH.:

The URL field is not shown?

cfenoy · 2016-02-22T14:18:08Z

We've hit also this issue. I understand we may want to provide more customization but we shouldn't do it if this breaks backwards compatibility.

boegel · 2016-02-22T14:20:23Z

From the looks of it, this now restores the previous behaviour of having Description: included in the default whatis line, without affecting custom whatis lines.

So, looks good to me, let's see if Jenkins agrees...

@geimer: your thoughts?

geimer · 2016-02-22T14:24:15Z

easybuild/tools/module_generator.py

        """
-        description = "%s - Homepage: %s" % (self.app.cfg['description'], self.app.cfg['homepage'])
+        description = "Description: %s - Homepage: %s" % (self.app.cfg['description'], self.app.cfg['homepage'])



Why not introduce the 'Description: ' prefix only for the whatis text (i.e., 3 lines below)? For the module help output it should be pretty clear that this is a description...

makes sense, good suggestion @geimer!

@pescobar: can you revert the change on this line, and change it below to:

whatis = ["Description: %s" % description]

hpcugentbot · 2016-02-22T14:25:45Z

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2747/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

geimer · 2016-02-22T14:33:29Z

@boegel: I still believe that the real issue is outside of EasyBuild. But I doubt that there will be a different short-term solution -- and maybe not even a long-term one...

hpcugentbot · 2016-02-22T14:34:48Z

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2748/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

hpcugentbot · 2016-02-22T14:52:08Z

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2749/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

hpcugentbot · 2016-02-22T15:01:47Z

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2750/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

boegel · 2016-02-22T15:17:40Z

test/framework/module_generator.py

-                "module-whatis {foo}",
-                "module-whatis {bar}",
+                "module-whatis {Description: foo}",
+                "module-whatis {Description: bar}",


the Description: part is not there is whatis is specified, so change this back to what it was

hpcugentbot · 2016-02-22T15:28:12Z

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2751/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

hpcugentbot · 2016-02-22T17:49:13Z

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2752/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 · 2016-02-23T08:39:26Z

Looks good to go to me... Any last words @geimer?

geimer · 2016-02-23T08:54:38Z

@boegel: Looks OK. It's amazing that 6 modified lines required 10 commits ;-)

boegel · 2016-02-23T08:55:17Z

Going in, thanks for tackling this @pescobar!

fix Lmod spider output in generated modules

fix Lmod spider output in generated modules

b1087ec

pescobar mentioned this pull request Jan 29, 2016

broken output for Lmod spider #1582

Closed

trying to fix the unit tests

d799d66

fix sanity checks. try2

04d6b47

wpoely86 reviewed Feb 8, 2016
View reviewed changes

wpoely86 added this to the v2.7.0 milestone Feb 8, 2016

pescobar added 2 commits February 22, 2016 15:16

undo previous change

e6ef641

correct fix, I hope :)

94676ff

geimer reviewed Feb 22, 2016
View reviewed changes

trying to fix unit tests

dc60287

change suggested by @geimer

f11781a

removed space

93f51e2

boegel reviewed Feb 22, 2016
View reviewed changes

trying to fix unit tests

b057174

add Description string also in lua modulefiles

6ac767d

boegel added a commit that referenced this pull request Feb 23, 2016

Merge pull request #1583 from pescobar/lmod-spider-output

96340f2

fix Lmod spider output in generated modules

boegel merged commit 96340f2 into easybuilders:develop Feb 23, 2016

Conversation

pescobar commented Jan 29, 2016

Uh oh!

hpcugentbot commented Jan 29, 2016

Uh oh!

hpcugentbot commented Jan 29, 2016

Uh oh!

hpcugentbot commented Jan 29, 2016

Uh oh!

wpoely86 Feb 8, 2016

Choose a reason for hiding this comment

Uh oh!

wpoely86 commented Feb 8, 2016

Uh oh!

wpoely86 commented Feb 8, 2016

Uh oh!

pescobar commented Feb 8, 2016

Uh oh!

hpcugentbot commented Feb 8, 2016

Uh oh!

wpoely86 commented Feb 8, 2016

Uh oh!

pescobar commented Feb 8, 2016

Uh oh!

wpoely86 commented Feb 8, 2016

Uh oh!

geimer commented Feb 8, 2016

Uh oh!

wpoely86 commented Feb 8, 2016

Uh oh!

boegel commented Feb 8, 2016

Uh oh!

wpoely86 commented Feb 8, 2016

Uh oh!

cfenoy commented Feb 22, 2016

Uh oh!

boegel commented Feb 22, 2016

Uh oh!

geimer Feb 22, 2016

Choose a reason for hiding this comment

Uh oh!

boegel Feb 22, 2016

Choose a reason for hiding this comment

Uh oh!

hpcugentbot commented Feb 22, 2016

Uh oh!

geimer commented Feb 22, 2016

Uh oh!

hpcugentbot commented Feb 22, 2016

Uh oh!

hpcugentbot commented Feb 22, 2016

Uh oh!

hpcugentbot commented Feb 22, 2016

Uh oh!

boegel Feb 22, 2016

Choose a reason for hiding this comment

Uh oh!

hpcugentbot commented Feb 22, 2016

Uh oh!

hpcugentbot commented Feb 22, 2016

Uh oh!

boegel commented Feb 23, 2016

Uh oh!

geimer commented Feb 23, 2016

Uh oh!

boegel commented Feb 23, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants