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

bugfix: lua-luafilesystem package should use stage.source_path #11648

Merged

Conversation

tgamblin
Copy link
Member

@tgamblin tgamblin commented Jun 6, 2019

Fixes #11645.
@odoublewen

After #11528, we use $stage_dir/src as the path for all expanded tarballs, so that the expanded path is known in advance. This helps client code that needs to know the checked out source path before fetching and expanding the source.

Some builds seem to assume they know stage.source_path and construct it themselves. e.g, lua-luafilesystem was doing this:

fmt = os.path.join(
    self.stage.path,
    'luafilesystem-{version.underscored}',
    'rockspecs',
    'luafilesystem-{semver.dotted}-{tweak_level}.rockspec'
)

It assumes the name of the directory under stage is 'luafilesystem-{version.underscored}'. Code like this should use stage.source_path instead of joining stage.path with an assumed name.

Here is the fix:

fmt = os.path.join(
    self.stage.source_path,
    'rockspecs',
    'luafilesystem-{semver.dotted}-{tweak_level}.rockspec'
)

@tgamblin
Copy link
Member Author

tgamblin commented Jun 6, 2019

@odoublewen: does this work for you?

@odoublewen
Copy link
Contributor

Works for me. Tried to do a quick survey of package.py files to see where else this construct might be lurking. Perhaps these lines in lmod/package.py also need to be updated? I'll test it....

    def setup_environment(self, spack_env, run_env):
        stage_lua_path = join_path(
            self.stage.path, 'Lmod-{version}', 'src', '?.lua')

@odoublewen
Copy link
Contributor

Well, the lmod package builds fine as it is...

@tgamblin
Copy link
Member Author

tgamblin commented Jun 6, 2019

I think it probably needs an update. Most of these as well:

$ git grep stage.path
packages/aspera-cli/package.py:        runfile = glob(join_path(self.stage.path, 'aspera-cli*.sh'))[0]
packages/bcl2fastq2/package.py:            with working_dir(self.stage.path):
packages/catalyst/package.py:        paraview_dir = os.path.join(self.stage.path,
packages/catalyst/package.py:            tty.msg("Generated catalyst source in %s" % self.stage.path)
packages/catalyst/package.py:                                                    self.stage.path))
packages/catalyst/package.py:        return os.path.join(self.stage.path, 'Catalyst-v' + str(self.version))
packages/charmpp/package.py:        make('-C', join_path(self.stage.path, 'charm/tests'),
packages/clhep/package.py:                    % (self.stage.path, self.spec.version))
packages/cuda/package.py:        runfile = glob(join_path(self.stage.path, 'cuda*_linux*'))[0]
packages/lmod/package.py:            self.stage.path, 'Lmod-{version}', 'src', '?.lua')
packages/nseg/package.py:                res_path = join_path(res.fetcher.stage.path, res.name)
packages/singularity/package.py:        return join_path(self.stage.path)

@tgamblin tgamblin merged commit 89b891d into develop Jun 6, 2019
@tgamblin tgamblin deleted the bugfix/packages-should-not-assume-expanded-tarball-name branch June 6, 2019 21:08
tldahlgren added a commit to tldahlgren/spack that referenced this pull request Jun 7, 2019
…#11648).

Note that installing `catalyst` fails in the same place it does for the commit prior to merging changes for spack#11528.
tldahlgren added a commit to tldahlgren/spack that referenced this pull request Jun 7, 2019
Fixes spack#11528
See also spack#11648

Note that installing `catalyst` fails in the same place it does for the commit prior to merging changes for spack#11528.
carsonwoods pushed a commit to carsonwoods/spack that referenced this pull request Jun 27, 2019
dev-zero pushed a commit to dev-zero/spack that referenced this pull request Aug 13, 2019
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.

Installation issue: lua-luafilesystem
3 participants