-
Notifications
You must be signed in to change notification settings - Fork 187
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
admin/ohpc-filesystem: include custom requires/provides plugin files for RPM dependency analysis #645
admin/ohpc-filesystem: include custom requires/provides plugin files for RPM dependency analysis #645
Conversation
Signed-off-by: Karl W. Schulz <karl.w.schulz@intel.com>
@adrianreber, these are updated dependency analysis files based on the TSC discussion. I removed any use of a TMPDIR, and added another command-line argument to |
@@ -91,6 +91,8 @@ Requires: llvm-compilers%{PROJ_DELIM} | |||
|
|||
%endif | |||
|
|||
%global __libsymlink_exclude_path %{OHPC_HOME}/.*$ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A comment what this for would be nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added in #648
@@ -0,0 +1,7 @@ | |||
%__ohpc_provides /usr/lib/rpm/ohpc-find-provides | |||
%__ohpc_requires /usr/lib/rpm/ohpc-find-requires %{buildroot} %{OHPC_HOME} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The usage of %{OHPC_HOME} in this file makes me a bit nervous. At what time is the file evaluated. After including OHPC_macros? But with happens with RPMs that do not define %{OHPC_HOME}. It then probably resolves to literally '%{OHPC_HOME}'. Is that a problem. Main question: Does this break building non-OHPC RPMS?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair point...let me look into that further and test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like standard macro syntax works fine for the path designations, so can guard those with the following:
%__ohpc_path ^%{!?OHPC_HOME:/opt/ohpc}
%__elf_exclude_path ^%{!?OHPC_HOME:/opt/ohpc}
The she-bang macro test does not look to work in the requires stanza, so working on a slightly amended solution for that. Will double check with ohpc and non-ohpc package builds and follow up with newer patches...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update in PR #648. See what you think. I built a non-ohpc package with the updated config and didn't see any issues (and get the same Provides/Requires results you get without the extra ohpc dependency files enabled).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, I take back the fact that the macro checks in the .attr file work as expected after kicking off a number of builds - I should have tested more with non-ohpc packages that include elf binaries. I have updated again via #650 and this approach simply calls out the /opt/ohpc
path directly instead of relying on macro definitions.
I was previously concerned with trying to allow for someone to relocate the top-level path for all ohpc packages (say customized environment were someone is rebuilding everything from .src rpms). However, in that case, a user will likely need to modify OHPC_macros to set the desired top-level path, so I just leveraged those settings in the creation of the ohpc.attr
file. If they want to customize, they can rebuild ohpc-buildroot
to update the paths to suit.
This includes several new files that are housed in
/usr/lib/rpm
to trigger the rpm dependency analysis plugin to use ohpc-provided scripts for analyzing ELF dependencies. These only get triggered for files installed into the%{OHPC_HOME}
path. This is one potential approach intended to avoid namespace collision with library .so's of the same name that might come from the underlying OS or other package repos (the example discussed in TSC meetings wasopenblas
).In this approach, an additional color of
(ohpc)
is appended to relevant Provides and Requires dependencies if the .so is housed within%{OHPC_HOME}
.