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: StageComposite must include the expanded property` #11647

Merged
merged 1 commit into from
Jun 6, 2019

Conversation

tgamblin
Copy link
Member

@tgamblin tgamblin commented Jun 6, 2019

After #11528, spack cd would fail like this in spack location because the StageComposite object didn't have all the necessary properties:

(spackbook):~$ spack cd lua-luafilesystem
==> Error: 'StageComposite' object has no attribute 'expanded'

Added the new expanded property to it.

@tgamblin
Copy link
Member Author

tgamblin commented Jun 6, 2019

Still needs a test

@mwkrentel
Copy link
Member

I use a package, intel-xed, that uses a resource. I used to get:

$ spack install intel-xed
...
==> Cloning git repository: https://github.com/intelxed/xed.git at commit b7231de4c808db821d64f4018d15412640c34113
==> Cloning git repository: https://github.com/intelxed/mbuild.git at commit 176544e1fb54b6bfb40f596111368981d287e951
==> No checksum needed when fetching with git
==> No checksum needed when fetching with git
==> Already staged intel-xed-2019.03.01-bg2qt4l553xtrwyey4usrwpqhd6hjnxk in /home/krentel/Cron/work.1/spack-repo/var/spack/stage/intel-xed-2019.03.01-bg2qt4l553xtrwyey4usrwpqhd6hjnxk
==> Already staged resource-mbuild-bg2qt4l553xtrwyey4usrwpqhd6hjnxk in /home/krentel/Cron/work.1/spack-repo/var/spack/stage/resource-mbuild-bg2qt4l553xtrwyey4usrwpqhd6hjnxk
==> Moving resource stage
        source : /home/krentel/Cron/work.1/spack-repo/var/spack/stage/resource-mbuild-bg2qt4l553xtrwyey4usrwpqhd6hjnxk/mbuild/
        destination : /home/krentel/Cron/work.1/spack-repo/var/spack/stage/intel-xed-2019.03.01-bg2qt4l553xtrwyey4usrwpqhd6hjnxk/xed/mbuild
==> No patches needed for intel-xed
==> Building intel-xed [Package]

But something in the last 24 hours has broken this. It no longer
reports 'Moving resource stage', and it fails like this:

$ spack install intel-xed
...
==> Cloning git repository: https://github.com/intelxed/xed.git at commit b7231de4c808db821d64f4018d15412640c34113
==> Cloning git repository: https://github.com/intelxed/mbuild.git at commit 176544e1fb54b6bfb40f596111368981d287e951
==> No checksum needed when fetching with git
==> No checksum needed when fetching with git
==> Already staged intel-xed-2019.03.01-enp62qcq63ixph45z3a5z2yudehcdana in /home/krentel/xed/spack-repo/var/spack/stage/intel-xed-2019.03.01-enp62qcq63ixph45z3a5z2yudehcdana
==> Already staged resource-mbuild-enp62qcq63ixph45z3a5z2yudehcdana in /home/krentel/xed/spack-repo/var/spack/stage/resource-mbuild-enp62qcq63ixph45z3a5z2yudehcdana
==> No patches needed for intel-xed
==> Building intel-xed [Package]
==> Executing phase: 'install'
==> [2019-06-06-14:30:55.094326] './mfile.py' '--clean'

XED build error: mfile.py cannot find the mbuild directory: [./../mbuild] or []

XED build error: mbuild import failed

I'm suspicious of #11528.
Is this the same problem?
Does this PR also fix the resource issue?

@tgamblin
Copy link
Member Author

tgamblin commented Jun 6, 2019

@mwkrentel: see #11648 for a better explanation of what changed -- @tldahlgren is looking into tweaking other packages.

@tldahlgren: can you look at @mwkrentel's issue above too?

@tgamblin tgamblin merged commit 0c13c3f into develop Jun 6, 2019
@tldahlgren
Copy link
Contributor

@tldahlgren: can you look at @mwkrentel's issue above too?

Sure.

@mwkrentel
Copy link
Member

@tldahlgren If it's a matter of adapting a resource or stage directory to
some new directory layout, I can do that. But I'd kinda like a heads up.

Intel-xed needs to set an environ variable to the location of the
resource directory. But the directory can exist anywhere, as long as
I can paste together a path name for it.

@tgamblin
Copy link
Member Author

tgamblin commented Jun 6, 2019

@mwkrentel: basically, if your package was previously trying to create stage.source_path before it existed by doing os.path.join(stage.path, 'mypackage-1.2') or similar, you should now just use stage.source_path, as it is known in advance. Use stage.expanded where you would've previously done if stage.source_path: ....

@mwkrentel
Copy link
Member

@tldahlgren I think what happened is that you used to move the
resource directory into the main package directory and now you don't.
Previously, I'd locate the resource directory as the subdir of the
source_path.

mbuild_dir = join_path(self.stage.source_path, 'mbuild')

So, self.stage.source_path gives me the main package directory, but
how do I find the location of the resource?

@tgamblin
Copy link
Member Author

tgamblin commented Jun 6, 2019

@mwkrentel: self.stage[1].path. I know it's not ideal -- they should really be indexed by name.

@tgamblin
Copy link
Member Author

tgamblin commented Jun 6, 2019

but we need to actually move the resource directories into the source path like we did before. That wasn't intended to change here.

@tgamblin
Copy link
Member Author

tgamblin commented Jun 6, 2019

Just a note -- develop failed weirdly in the Python 2.6 tests after this was merged having passed the tests:

But subsequent develop builds appear to be working, so I am not sure it is related. Linking the offending Travis build here for reference.

@mwkrentel
Copy link
Member

but we need to actually move the resource directories into the
source path like we did before. That wasn't intended to change here.

Ok, if that's the plan, that you'll copy the resource directories
inside the main package directory with the same subdir name as before,
then I think that will transparently fix my problem.

I'm guessing that means that pretty much every package that uses a
resource is currently broken for the same reason?

@tgamblin tgamblin deleted the bugfix/staging-issues branch June 7, 2019 17:24
tldahlgren added a commit to tldahlgren/spack that referenced this pull request Jun 7, 2019
tldahlgren added a commit to tldahlgren/spack that referenced this pull request Jun 7, 2019
See spack#11647
See spack#11528

Also added (missing) DIYStage unit tests to improve overall code coverage.
tldahlgren added a commit to tldahlgren/spack that referenced this pull request Jun 7, 2019
tldahlgren added a commit to tldahlgren/spack that referenced this pull request Jun 7, 2019
See spack#11647
See spack#11528

Also added (missing) DIYStage unit tests to improve overall code coverage.
@tldahlgren
Copy link
Contributor

@tldahlgren I think what happened is that you used to move the
resource directory into the main package directory and now you don't.
Previously, I'd locate the resource directory as the subdir of the
source_path.

mbuild_dir = join_path(self.stage.source_path, 'mbuild')

Thanks for pointing this out.

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.

None yet

3 participants