-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
Fix invalid paddle binary file path #3421
Merged
Merged
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
dfc86dd
Fix invalid paddle binary file path
Yancey1989 98255dc
update
Yancey1989 a87b82c
update
Yancey1989 d6bd503
merge develop branch
Yancey1989 98b35ab
fix paddle bin directory
Yancey1989 d98e2d5
update install doc
Yancey1989 5c926b0
update
Yancey1989 a872e33
update
Yancey1989 ec9ac91
update
Yancey1989 3565518
update
Yancey1989 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,13 +24,14 @@ if '${CMAKE_SYSTEM_PROCESSOR}' not in ['arm', 'armv7-a', 'aarch64']: | |
setup_requires+=["opencv-python"] | ||
|
||
# the prefix is sys.prefix which should always be usr | ||
paddle_bin_dir = 'local/opt/paddle/bin' | ||
paddle_bin_dir = 'opt/paddle/bin' | ||
paddle_bins = ['${PADDLE_BINARY_DIR}/paddle/scripts/paddle_usage', | ||
'${PADDLE_BINARY_DIR}/paddle/trainer/paddle_trainer', | ||
'${PADDLE_BINARY_DIR}/paddle/trainer/paddle_merge_model', | ||
'${PADDLE_BINARY_DIR}/paddle/pserver/paddle_pserver_main'] | ||
'${PADDLE_BINARY_DIR}/paddle/pserver/paddle_pserver_main', | ||
'${PADDLE_BINARY_DIR}/paddle/scripts/paddle'] | ||
|
||
paddle_rt_lib_dir = 'local/lib' | ||
paddle_rt_lib_dir = 'lib' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this change would impact "pip install " without docker for user. Anyway, if |
||
paddle_rt_libs = [] if '${MKL_SHARED_LIBS}'== '' else '${MKL_SHARED_LIBS}'.split(';') | ||
|
||
setup(name='paddlepaddle', | ||
|
@@ -50,8 +51,7 @@ setup(name='paddlepaddle', | |
'paddle.v2.framework.proto': '${PADDLE_BINARY_DIR}/paddle/framework', | ||
'py_paddle': '${PADDLE_SOURCE_DIR}/paddle/py_paddle' | ||
}, | ||
scripts=['${PADDLE_BINARY_DIR}/paddle/scripts/paddle'], | ||
scripts=paddle_bins, | ||
distclass=BinaryDistribution, | ||
data_files=[(paddle_bin_dir, paddle_bins), | ||
(paddle_rt_lib_dir, paddle_rt_libs)] | ||
data_files=[(paddle_rt_lib_dir, paddle_rt_libs)] | ||
) |
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.
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 cause difference path when use
cmake install
.cmake install
:/usr/local/opt/paddle/bin
pip install
:/usr/opt/paddle/bin
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 @tensor-tang , but maybe it doesn't matter for the different path?
If users want to install paddle to a customer directory, he/she could set
--prefix=<customer directory>
, but in the production Docker image, we just make it running correctly.BTW, the directory in
date_files
wouldn't be configuredlocal/opt/paddle/bin
, this will caused paddle be installed under/usr/local/local/opt/paddle/bin
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.
@tensor-tang ,
paddle_bin_files
is executable files, so I move these toscripts
int setup.py.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 think
make install
andpip install
will install to different default location doesn't harm using.make install
orpip install
to install and then use paddle after that.--prefix
parameter when installing, and the default installation behaviour?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 am not sure about this, maybe @luotao1 @typhoonzero can answer this?
Totally agreed~
Anyway, I am OK with the both ways, just giving a method for this case.
You can change them as you wish. I think the efficient way is the best~
Thanks @Yancey1989 @typhoonzero ~
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 with the document for installing PaddlePaddle.
And thanks @typhoonzero and @tensor-tang !!