-
Notifications
You must be signed in to change notification settings - Fork 41
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
Add flag to move libraries under project name #188
Conversation
1536c0f
to
d186f0c
Compare
@@ -119,7 +119,7 @@ endfunction() | |||
option(ROCM_SYMLINK_LIBS "Create backwards compatibility symlink for library files." ON) | |||
|
|||
function(rocm_install_targets) | |||
set(options) | |||
set(options PRIVATE_LIB) |
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.
Can the word PRIVATE
work here?
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.
Done.
@@ -131,21 +131,51 @@ function(rocm_install_targets) | |||
set(EXPORT_FILE ${PARSE_EXPORT}) | |||
endif() | |||
|
|||
set(PRIVATE_LIB OFF) |
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.
No need to make an intermediate variable, you can just check if(PARSE_PRIVATE)
everywhere.
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.
I have made all the requested changes but I seem to be running into test failures now. Not sure if this change is causing the issue or is it something else that I am missing, but my changes are now breaking CI. I am looking into it.
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.
I have fixed the CI failures.
test/libprivate/SimplePrivate.cpp
Outdated
@@ -0,0 +1,7 @@ | |||
/******************************************************************************* |
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.
Source files should be all lowercase.
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.
Done.
#ifndef GUARD_SIMPLE_PRIVATE_H | ||
#define GUARD_SIMPLE_PRIVATE_H | ||
|
||
void simple-private(); |
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.
This should be simple_private
as -
is not valid symbol identifier.
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.
Thanks for catching that, I missed to change it here. Done.
2673381
to
055d869
Compare
258e82d
to
cbff664
Compare
b3a73a7
to
714b163
Compare
Libraries under Ubuntu:
SLES:
|
The CI failures seem unrelated to my changes. |
List of files from package... Ubuntu:
dev:
|
List of files from package... SLES:
dev:
|
Results of make install and make package are with the following changes to MIGraphX:
|
Add flag to move libraries and include files under project name.