Don't inject import statements in package recipes#47947
Merged
tgamblin merged 1 commit intospack:developfrom Dec 5, 2024
Merged
Don't inject import statements in package recipes#47947tgamblin merged 1 commit intospack:developfrom
tgamblin merged 1 commit intospack:developfrom
Conversation
Remove a hack done by RepoLoader, which was injecting an extra ``` from spack.package import * ``` at the beginning of each package.py
spackbot-app
bot
added
commands
core
Member
|
I mean you can say it was done by me since I did it -- no need to anthropomorphize |
tgamblin
approved these changes
Dec 5, 2024
Member
Author
Or I can say that after 9 p.m. my English writing skills are 📉 😆 |
7 tasks
wdconinc
added a commit
to eic/eic-spack
that referenced
this pull request
Dec 7, 2024
### Briefly, what does this PR introduce? This PR ensures that all packages have a `from spack.package import *` line since it is not injected by spack anymore. Ref: spack/spack#47947 ### What kind of change does this PR introduce? - [x] Bug fix (issue #__) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? No.
Contributor
|
Ref #39380 (comment). There still was no actual deprecation warning printed by spack on import of packages without this import, so this may still surprise many 3rd party repo owners. |
Member
Author
|
@wdconinc On the other hand: should fix them automatically. Should we catch that kind of error, and suggest to run the command? |
Member
|
Can we add something to the importer to warn that the import is needed? |
Contributor
|
With The exception appears as a diff --git a/lib/spack/spack/repo.py b/lib/spack/spack/repo.py
index c4a104c6b9..be5234898b 100644
--- a/lib/spack/spack/repo.py
+++ b/lib/spack/spack/repo.py
@@ -1240,6 +1240,13 @@ def get_pkg_class(self, pkg_name: str) -> Type["spack.package_base.PackageBase"]
module = importlib.import_module(fullname)
except ImportError:
raise UnknownPackageError(fullname)
+ except NameError as e:
+ msg = (
+ f"cannot load package '{pkg_name}' from the '{self.namespace}' repository: {e} " +
+ f"(this may be due to missing 'from spack.package import *' in {pkg_name}, which " +
+ f"can be fixed by running 'spack style --fix')"
+ )
+ raise RepoError(msg) from e
except Exception as e:
msg = f"cannot load package '{pkg_name}' from the '{self.namespace}' repository: {e}"
raise RepoError(msg) from e(Not sure how to get a filename from the info we have without creating the potential for more exceptions.) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Remove a hack done
byinRepoLoader, which was injecting an extraat the beginning of each
package.py.