resource directive : implementation + clang / llvm use case

[alalazo]

Changeset:

• added resource directive
• modified clang / llvm to enable fetch of resources
• libc++ variant for llvm
• mirror fetches resources

Notes:

• mirror won't work for sources fetched from repositories
• glitches in the default paths for system headers (OpenMP, libc++)

[alalazo]

I am opening a PR early to receive advice particularly on:

• wrong or non-idiomatic implementation
• missing error handling
• missing bits in the implementation

[alalazo]

Would it be fine to have a factory in fetch_strategy.py that accepts and parses kwargs and returns the appropriate strategy? If so should it be a static method of FetchStrategy? I mean something like:

class FetchStrategy(object):
    @staticmethod
    def create(**kwargs):
        # Selects strategy based on required arguments
        return strategy

The idea would be to handle cases in which resources are checked-out from repositories, e.g. if a support for @trunk will be added to clang.

[tgamblin]

There's already something like this in fetch_strategy.py -- it's the for_package_version method. It's more complicated than you need here because it allows you to supply some default args in the actual package class, e.g.:

    class Foo(Package):
        git = 'http://example.com/repo.git'

        version('2.6', tag='v2.6')
        version('2.7', tag='v2.7')

That should actually work and it'll instantiate git fetchers for both versions. For resources, that's not necessary (or desired) so I think a factory would work well. You could either do your static method, or you could add something like from_args() down where from_url and for_package_version are.

I think the only code you really need is this, from the middle of for_package_version:

    for fetcher in all_strategies:                                                                                         
        if fetcher.matches(args):                                                                                          
            return fetcher(**args)   

I believe I had a simpler version of this in its own method before, but it wasn't used anywhere so I just left it out rather than having dead code.

[alalazo]

@tgamblin : I tried to change the code according to the comments. Please tell me if you see something that was not fixed appropriately.

A couple of minor questions:

---

In fetch_strategy.py the matches method uses any:

    # This method is used to match fetch strategies to version()
    # arguments in packages.
    @classmethod
    def matches(cls, args):
        return any(k in args for k in cls.required_attributes)

I was wondering if all shouldn't be used here. Right now it's unimportant, as every "concrete" class has one required attribute, but it may matter as soon as a class with 2 or more required attributes is added.

---

How should we treat build-time only dependencies? For instance llvm needs cmake to be built, and in machines where it's not installed the installation fails with the obscure message (for a user who doesn't know python):

TypeError: 'NoneType' object is not callable

Should we in general add a dependency on cmake to packages that use it?

[alalazo]

Up to now, I was not able to set up the paths for some system headers correctly. I described the issue here for OpenMP:

http://lists.llvm.org/pipermail/cfe-users/2015-December/000832.html

but the same holds true for C++ standard headers if libc++ is compiled. The issue seems to show up when llvm and clang are compiled separately.

[alalazo]

@tgamblin documentation, tests and maybe a bit of refactoring to remove duplicated logic are still on my todo list. Do you prefer to continue working within this PR or should we open another one later for those things?

[tgamblin]

@alalazo: I think we can commit this one and open another for the other stuff.

[tgamblin]

FYI: It's on my todo list to allow referencing the package's own version in URLs, patch paths, etc. So you could write something like this:

http://llvm.org/releases/${spec.version}/compiler-rt-${spec.version}.src.tar.xz

I need to think about how to implement it but it shouldn't be so hard. I just want to get the syntax right. Not sure ${} is the best way to substitute it, either.

[tgamblin]

@alalazo: do you consider this PR ready to merge? It looks good to me -- I'll test installing clang@3.7.0. Is there anything else you want to add?

[alalazo]

@tgamblin : I don't have objections merging this PR as it is, but I think there are two things you may want to consider adding / fixing:

• here I extended the resource directive keyword argument basename to be a dictionary to have a finer grained control over the linking of resource directories. I could cherry-pick that modification in this PR. If you check how GCC is built in that branch, you'll see an example of use (binutils).
• the configuration of CLang still isn't optimal (see the link in a comment above for a full description of the problem). Everything works, but you have to pass extra include flags that shouldn't be needed.

[tgamblin]

@alalazo: I like the basename thing but is there a more intuitive name? It's basically saying to put the file in the expanded archive or the stage dir, right? Are those the only two options someone would want? Could you get the same effect with just ../ on the destination?

I think we should fix the 2nd thing before merging -- the clang configuration shouldn't require extra includes. Can you let me know when that is done?

[alalazo]

@tgamblin :

1. I changed the keyword basename into placement (hope its more descriptive, I am open for better suggestions). Basically :
   • the resources are staged and extracted into their own folder, then resource.source_path is linked in the appropriate subtree of self.stage.source_path (check Package.do_stage)
   • destination is an optional subpath of self.stage.source_path where to link the resource
   • placement is optional and is either a string or a dictionary. If it is a string it links resource.source_path under a different name. If it is a dictionary it maps a subfolder of resource.source_path to a subfolder of self.source_path/destination. The string case comes from LLVM and CLang, the dictionary from GCC in-tree build with binutils.
2. the problem with the headers should not be there anymore. I linked the apropriate llvm include folders where clang expects them. I don't think it's something spack should do, but I found no other way of having clang handling this when its built with an external llvm.

As soon as you confirm the compilation goes fine and the include path issue has disappeared it, I think the PR could be merged.