You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mxnet.apache.org by GitBox <gi...@apache.org> on 2020/03/11 15:34:40 UTC
[GitHub] [incubator-mxnet] sl1pkn07 opened a new pull request #17815: Add
SOVERSION when build shared libmxnet.so library
sl1pkn07 opened a new pull request #17815: Add SOVERSION when build shared libmxnet.so library
URL: https://github.com/apache/incubator-mxnet/pull/17815
## Description ##
https://en.wikipedia.org/wiki/Soname
https://cmake.org/cmake/help/latest/prop_tgt/SOVERSION.html
## Checklist ##
### Essentials ###
Please feel free to remove inapplicable items for your PR.
- [ ] The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant [JIRA issue](https://issues.apache.org/jira/projects/MXNET/issues) created (except PRs with tiny changes)
- [x] Changes are complete (i.e. I finished coding on this PR)
- [ ] All changes have test coverage:
- Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
- Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
- Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
- [ ] Code is well-documented:
- For user-facing API changes, API doc string has been updated.
- For new C++ functions in header files, their functionalities and arguments are documented.
- For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
- Check the API doc at https://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
- [ ] To the best of my knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change
### Changes ###
- [ ] Feature1, tests, (and when applicable, API doc)
- [ ] Feature2, tests, (and when applicable, API doc)
## Comments ##
- If this change is a backward incompatible change, why must this change be made.
- Interesting edge cases to note here
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on issue #17815: Add SOVERSION
when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
leezu commented on issue #17815: Add SOVERSION when build shared libmxnet.so library
URL: https://github.com/apache/incubator-mxnet/pull/17815#issuecomment-597773010
You may need to update the CI settings to be more flexible about the name of the `.so` file:
See the locations here https://github.com/apache/incubator-mxnet/search?q=mx_lib&unscoped_q=mx_lib
Further you'll need to update the staticbuild script https://github.com/apache/incubator-mxnet/blob/0f29ccaf5c34a01bffe5ad3036d4cf8bb00c551e/tools/staticbuild/build_lib_cmake.sh and perhaps the setup.py for the staticbuild https://github.com/apache/incubator-mxnet/tree/0f29ccaf5c34a01bffe5ad3036d4cf8bb00c551e/tools/pip
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] sl1pkn07 commented on a change in pull request
#17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
sl1pkn07 commented on a change in pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
URL: https://github.com/apache/incubator-mxnet/pull/17815#discussion_r394377382
##########
File path: CMakeLists.txt
##########
@@ -704,6 +712,8 @@ if(UNIX)
target_link_libraries(mxnet PRIVATE mxnet_static)
target_link_libraries(mxnet_static PUBLIC ${CMAKE_DL_LIBS})
set_target_properties(mxnet_static PROPERTIES OUTPUT_NAME mxnet)
+ set_target_properties(mxnet PROPERTIES VERSION "${ver_major}.${ver_minor}.${ver_patch}")
Review comment:
other test, and this surprise me, i just add
~~~cmake
message(STATUS "Project version: ${PROJECT_VERSION}")
~~~
just before `project()` function, and works
but is vanished if put that in the bottom/middle of the CMakeLists
cmake is a bit complex for me, IDK where is the problem, so, i think is better keep untounch with `${ver_major}.${ver_minor}.${ver_patch}` and in near future (when i found the problem), fix it
greetings
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on a change in pull request
#17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
URL: https://github.com/apache/incubator-mxnet/pull/17815#discussion_r394042163
##########
File path: CMakeLists.txt
##########
@@ -1,5 +1,14 @@
cmake_minimum_required(VERSION 3.13)
+file(READ "include/mxnet/base.h" ver)
+string(REGEX MATCH "MXNET_MAJOR ([0-9]*)" _ ${ver})
+set(ver_major ${CMAKE_MATCH_1})
+string(REGEX MATCH "MXNET_MINOR ([0-9]*)" _ ${ver})
+set(ver_minor ${CMAKE_MATCH_1})
+string(REGEX MATCH "MXNET_PATCH ([0-9]*)" _ ${ver})
+set(ver_patch ${CMAKE_MATCH_1})
+set(_PROJECT_VERSION "${ver_major}.${ver_minor}.${ver_patch}")
Review comment:
Why set `_PROJECT_VERSION`?
Instead you may want to use something like `project(mxnet C CXX VERSION "${ver_major}.${ver_minor}.${ver_patch}")` instead of the existing `project(mxnet C CXX)`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] sl1pkn07 commented on a change in pull request
#17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
sl1pkn07 commented on a change in pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
URL: https://github.com/apache/incubator-mxnet/pull/17815#discussion_r394398634
##########
File path: CMakeLists.txt
##########
@@ -704,6 +712,8 @@ if(UNIX)
target_link_libraries(mxnet PRIVATE mxnet_static)
target_link_libraries(mxnet_static PUBLIC ${CMAKE_DL_LIBS})
set_target_properties(mxnet_static PROPERTIES OUTPUT_NAME mxnet)
+ set_target_properties(mxnet PROPERTIES VERSION "${ver_major}.${ver_minor}.${ver_patch}")
Review comment:
i think is finally done
tested on local and works as spected
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on a change in pull request
#17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
URL: https://github.com/apache/incubator-mxnet/pull/17815#discussion_r394537410
##########
File path: CMakeLists.txt
##########
@@ -704,6 +712,8 @@ if(UNIX)
target_link_libraries(mxnet PRIVATE mxnet_static)
target_link_libraries(mxnet_static PUBLIC ${CMAKE_DL_LIBS})
set_target_properties(mxnet_static PROPERTIES OUTPUT_NAME mxnet)
+ set_target_properties(mxnet PROPERTIES VERSION "${ver_major}.${ver_minor}.${ver_patch}")
Review comment:
Thank you for fixing it
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on a change in pull request
#17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
URL: https://github.com/apache/incubator-mxnet/pull/17815#discussion_r394111061
##########
File path: CMakeLists.txt
##########
@@ -704,6 +712,8 @@ if(UNIX)
target_link_libraries(mxnet PRIVATE mxnet_static)
target_link_libraries(mxnet_static PUBLIC ${CMAKE_DL_LIBS})
set_target_properties(mxnet_static PROPERTIES OUTPUT_NAME mxnet)
+ set_target_properties(mxnet PROPERTIES VERSION "${ver_major}.${ver_minor}.${ver_patch}")
Review comment:
You don't need the quotation marks around ${PROJECT_VERSION}
Ref: Github search: https://github.com/search?l=CMake&q=set_target_properties%28+PROPERTIES+VERSION+PROJECT_VERSION%29&type=Code
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] sl1pkn07 commented on a change in pull request
#17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
sl1pkn07 commented on a change in pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
URL: https://github.com/apache/incubator-mxnet/pull/17815#discussion_r394362027
##########
File path: CMakeLists.txt
##########
@@ -704,6 +712,8 @@ if(UNIX)
target_link_libraries(mxnet PRIVATE mxnet_static)
target_link_libraries(mxnet_static PUBLIC ${CMAKE_DL_LIBS})
set_target_properties(mxnet_static PROPERTIES OUTPUT_NAME mxnet)
+ set_target_properties(mxnet PROPERTIES VERSION "${ver_major}.${ver_minor}.${ver_patch}")
Review comment:
without quotes
<details>
</details>
with quotes
<details>
</details>
if you see some examples you paste, `PROJECT_VERSION` is ser as sis before
https://github.com/fps/ladspa.m.plugins/blob/3d595382b72674e926fb52a426fa7689ce1aa999/CMakeLists.txt#L8
https://github.com/devcrmbmp812/QTwork/blob/56129534067aa2bc0875e538ce91f8e32ae84e72/CMakeLists.txt#L6
the only case i test and works is https://github.com/eyjohn/scratch-binary-compat/blob/master/CMakeLists.txt
but this, as you see in the captures, not works on mxnet
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] leezu edited a comment on pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
leezu edited a comment on pull request #17815:
URL: https://github.com/apache/incubator-mxnet/pull/17815#issuecomment-665272450
@mxnet-bot run ci [all]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] sl1pkn07 commented on a change in pull request
#17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
sl1pkn07 commented on a change in pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
URL: https://github.com/apache/incubator-mxnet/pull/17815#discussion_r394362027
##########
File path: CMakeLists.txt
##########
@@ -704,6 +712,8 @@ if(UNIX)
target_link_libraries(mxnet PRIVATE mxnet_static)
target_link_libraries(mxnet_static PUBLIC ${CMAKE_DL_LIBS})
set_target_properties(mxnet_static PROPERTIES OUTPUT_NAME mxnet)
+ set_target_properties(mxnet PROPERTIES VERSION "${ver_major}.${ver_minor}.${ver_patch}")
Review comment:
without quotes
<details>
</details>
with quotes
<details>
</details>
if you see some examples you paste, `PROJECT_VERSION` is set as-sis before
https://github.com/fps/ladspa.m.plugins/blob/3d595382b72674e926fb52a426fa7689ce1aa999/CMakeLists.txt#L8
https://github.com/devcrmbmp812/QTwork/blob/56129534067aa2bc0875e538ce91f8e32ae84e72/CMakeLists.txt#L6
the only case i test and works is https://github.com/eyjohn/scratch-binary-compat/blob/master/CMakeLists.txt
but this, as you see in the captures, not works on mxnet
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] sl1pkn07 commented on a change in pull request
#17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
sl1pkn07 commented on a change in pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
URL: https://github.com/apache/incubator-mxnet/pull/17815#discussion_r394383299
##########
File path: CMakeLists.txt
##########
@@ -704,6 +712,8 @@ if(UNIX)
target_link_libraries(mxnet PRIVATE mxnet_static)
target_link_libraries(mxnet_static PUBLIC ${CMAKE_DL_LIBS})
set_target_properties(mxnet_static PROPERTIES OUTPUT_NAME mxnet)
+ set_target_properties(mxnet PROPERTIES VERSION "${ver_major}.${ver_minor}.${ver_patch}")
Review comment:
ook, i found the problem
`project()` is call twice in the main CMakeFile
https://github.com/apache/incubator-mxnet/blob/master/CMakeLists.txt#L9
https://github.com/apache/incubator-mxnet/blob/master/CMakeLists.txt#L89
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] sl1pkn07 commented on a change in pull request
#17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
sl1pkn07 commented on a change in pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
URL: https://github.com/apache/incubator-mxnet/pull/17815#discussion_r394047744
##########
File path: CMakeLists.txt
##########
@@ -1,5 +1,14 @@
cmake_minimum_required(VERSION 3.13)
+file(READ "include/mxnet/base.h" ver)
+string(REGEX MATCH "MXNET_MAJOR ([0-9]*)" _ ${ver})
+set(ver_major ${CMAKE_MATCH_1})
+string(REGEX MATCH "MXNET_MINOR ([0-9]*)" _ ${ver})
+set(ver_minor ${CMAKE_MATCH_1})
+string(REGEX MATCH "MXNET_PATCH ([0-9]*)" _ ${ver})
+set(ver_patch ${CMAKE_MATCH_1})
+set(_PROJECT_VERSION "${ver_major}.${ver_minor}.${ver_patch}")
Review comment:
missing "language"
https://cmake.org/cmake/help/latest/command/project.html
noy im not sure if need other changes
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] leezu edited a comment on issue #17815: Add
SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
leezu edited a comment on issue #17815: Add SOVERSION when build shared libmxnet.so library
URL: https://github.com/apache/incubator-mxnet/pull/17815#issuecomment-597773010
You may need to update the CI settings to be more flexible about the name of the `.so` file:
See the locations here https://github.com/apache/incubator-mxnet/search?q=mx_lib&unscoped_q=mx_lib CI jobs will use these `mx_lib` etc lists to decide on the files to copy save from the build and to re-instantiate for the unittests.
Further you'll need to update the staticbuild script https://github.com/apache/incubator-mxnet/blob/0f29ccaf5c34a01bffe5ad3036d4cf8bb00c551e/tools/staticbuild/build_lib_cmake.sh and perhaps the setup.py for the staticbuild https://github.com/apache/incubator-mxnet/tree/0f29ccaf5c34a01bffe5ad3036d4cf8bb00c551e/tools/pip
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] sl1pkn07 commented on a change in pull request
#17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
sl1pkn07 commented on a change in pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
URL: https://github.com/apache/incubator-mxnet/pull/17815#discussion_r394047949
##########
File path: CMakeLists.txt
##########
@@ -704,6 +712,8 @@ if(UNIX)
target_link_libraries(mxnet PRIVATE mxnet_static)
target_link_libraries(mxnet_static PUBLIC ${CMAKE_DL_LIBS})
set_target_properties(mxnet_static PROPERTIES OUTPUT_NAME mxnet)
+ set_target_properties(mxnet PROPERTIES VERSION "${ver_major}.${ver_minor}.${ver_patch}")
Review comment:
needs this with the last change?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on a change in pull request
#17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
URL: https://github.com/apache/incubator-mxnet/pull/17815#discussion_r392416938
##########
File path: ci/jenkins/Jenkins_steps.groovy
##########
@@ -23,27 +23,27 @@
utils = load('ci/Jenkinsfile_utils.groovy')
// mxnet libraries
-mx_lib = 'build/libmxnet.so, build/3rdparty/tvm/libtvm_runtime.so, build/libtvmop.so, build/tvmop.conf, build/libcustomop_lib.so, build/libcustomop_gpu_lib.so, build/libsubgraph_lib.so, build/3rdparty/openmp/runtime/src/libomp.so'
-mx_lib_cython = 'build/libmxnet.so, build/3rdparty/tvm/libtvm_runtime.so, build/libtvmop.so, build/tvmop.conf, build/libcustomop_lib.so, build/libcustomop_gpu_lib.so, build/libsubgraph_lib.so, python/mxnet/_cy3/*.so, build/3rdparty/openmp/runtime/src/libomp.so, python/mxnet/_ffi/_cy3/*.so'
+mx_lib = 'build/libmxnet.so*, build/3rdparty/tvm/libtvm_runtime.so, build/libtvmop.so, build/tvmop.conf, build/libcustomop_lib.so, build/libcustomop_gpu_lib.so, build/libsubgraph_lib.so, build/3rdparty/openmp/runtime/src/libomp.so'
+mx_lib_cython = 'build/libmxnet.so*, build/3rdparty/tvm/libtvm_runtime.so, build/libtvmop.so, build/tvmop.conf, build/libcustomop_lib.so, build/libcustomop_gpu_lib.so, build/libsubgraph_lib.so, python/mxnet/_cy3/*.so, build/3rdparty/openmp/runtime/src/libomp.so, python/mxnet/_ffi/_cy3/*.so'
mx_lib_make = 'lib/libmxnet.so, lib/libmxnet.a, lib/libtvm_runtime.so, lib/libtvmop.so, lib/tvmop.conf, build/libcustomop_lib.so, build/libcustomop_gpu_lib.so, build/libsubgraph_lib.so, 3rdparty/dmlc-core/libdmlc.a, 3rdparty/tvm/nnvm/lib/libnnvm.a'
// Python wheels
mx_pip = 'build/*.whl'
// mxnet cmake libraries, in cmake builds we do not produce a libnvvm static library by default.
-mx_cmake_lib = 'build/libmxnet.so, build/3rdparty/tvm/libtvm_runtime.so, build/libtvmop.so, build/tvmop.conf, build/tests/mxnet_unit_tests, build/3rdparty/openmp/runtime/src/libomp.so'
-mx_cmake_lib_no_tvm_op = 'build/libmxnet.so, build/libcustomop_lib.so, build/libcustomop_gpu_lib.so, build/libsubgraph_lib.so, build/tests/mxnet_unit_tests, build/3rdparty/openmp/runtime/src/libomp.so'
+mx_cmake_lib = 'build/libmxnet.so*, build/3rdparty/tvm/libtvm_runtime.so, build/libtvmop.so, build/tvmop.conf, build/tests/mxnet_unit_tests, build/3rdparty/openmp/runtime/src/libomp.so'
+mx_cmake_lib_no_tvm_op = 'build/libmxnet.so*, build/libcustomop_lib.so, build/libcustomop_gpu_lib.so, build/libsubgraph_lib.so, build/tests/mxnet_unit_tests, build/3rdparty/openmp/runtime/src/libomp.so'
mx_cmake_lib_cython = 'build/libmxnet.so, build/3rdparty/tvm/libtvm_runtime.so, build/libtvmop.so, build/tvmop.conf, build/tests/mxnet_unit_tests, build/3rdparty/openmp/runtime/src/libomp.so, python/mxnet/_cy3/*.so, python/mxnet/_ffi/_cy3/*.so'
// mxnet cmake libraries, in cmake builds we do not produce a libnvvm static library by default.
-mx_cmake_lib_debug = 'build/libmxnet.so, build/3rdparty/tvm/libtvm_runtime.so, build/libtvmop.so, build/tvmop.conf, build/libcustomop_lib.so, build/libcustomop_gpu_lib.so, build/libsubgraph_lib.so, build/tests/mxnet_unit_tests'
-mx_mkldnn_lib = 'build/libmxnet.so, build/3rdparty/tvm/libtvm_runtime.so, build/libtvmop.so, build/tvmop.conf, build/3rdparty/openmp/runtime/src/libomp.so, build/libcustomop_lib.so, build/libcustomop_gpu_lib.so, build/libsubgraph_lib.so'
-mx_mkldnn_lib_make = 'lib/libmxnet.so, lib/libmxnet.a, lib/libtvm_runtime.so, lib/libtvmop.so, lib/tvmop.conf, build/libcustomop_lib.so, build/libcustomop_gpu_lib.so, build/libsubgraph_lib.so, 3rdparty/dmlc-core/libdmlc.a, 3rdparty/tvm/nnvm/lib/libnnvm.a'
-mx_tensorrt_lib = 'build/libmxnet.so, build/3rdparty/tvm/libtvm_runtime.so, build/libtvmop.so, build/tvmop.conf, build/3rdparty/openmp/runtime/src/libomp.so, lib/libnvonnxparser_runtime.so.0, lib/libnvonnxparser.so.0, lib/libonnx_proto.so, lib/libonnx.so'
-mx_lib_cpp_examples = 'build/libmxnet.so, build/3rdparty/tvm/libtvm_runtime.so, build/libtvmop.so, build/tvmop.conf, build/3rdparty/openmp/runtime/src/libomp.so, build/libcustomop_lib.so, build/libcustomop_gpu_lib.so, build/libsubgraph_lib.so, build/cpp-package/example/*, python/mxnet/_cy3/*.so, python/mxnet/_ffi/_cy3/*.so'
-mx_lib_cpp_examples_make = 'lib/libmxnet.so, lib/libmxnet.a, lib/libtvm_runtime.so, lib/libtvmop.so, lib/tvmop.conf, build/libcustomop_lib.so, build/libcustomop_gpu_lib.so, build/libsubgraph_lib.so, 3rdparty/dmlc-core/libdmlc.a, 3rdparty/tvm/nnvm/lib/libnnvm.a, 3rdparty/ps-lite/build/libps.a, deps/lib/libprotobuf-lite.a, deps/lib/libzmq.a, build/cpp-package/example/*, python/mxnet/_cy3/*.so, python/mxnet/_ffi/_cy3/*.so'
-mx_lib_cpp_capi_make = 'lib/libmxnet.so, lib/libmxnet.a, lib/libtvm_runtime.so, lib/libtvmop.so, lib/tvmop.conf, libsample_lib.so, lib/libmkldnn.so.1, lib/libmklml_intel.so, 3rdparty/dmlc-core/libdmlc.a, 3rdparty/tvm/nnvm/lib/libnnvm.a, 3rdparty/ps-lite/build/libps.a, deps/lib/libprotobuf-lite.a, deps/lib/libzmq.a, build/cpp-package/example/*, python/mxnet/_cy3/*.so, python/mxnet/_ffi/_cy3/*.so, build/tests/cpp/mxnet_unit_tests'
-mx_lib_cpp_examples_no_tvm_op = 'build/libmxnet.so, build/libcustomop_lib.so, build/libcustomop_gpu_lib.so, build/libsubgraph_lib.so, build/3rdparty/openmp/runtime/src/libomp.so, build/cpp-package/example/*, python/mxnet/_cy3/*.so, python/mxnet/_ffi/_cy3/*.so'
-mx_lib_cpp_examples_cpu = 'build/libmxnet.so, build/3rdparty/tvm/libtvm_runtime.so, build/libtvmop.so, build/tvmop.conf, build/3rdparty/openmp/runtime/src/libomp.so, build/cpp-package/example/*'
+mx_cmake_lib_debug = 'build/libmxnet.so*, build/3rdparty/tvm/libtvm_runtime.so, build/libtvmop.so, build/tvmop.conf, build/libcustomop_lib.so, build/libcustomop_gpu_lib.so, build/libsubgraph_lib.so, build/tests/mxnet_unit_tests'
Review comment:
Unrelated to your PR, but potential improvement: Have only one variable for makefile build and one for cmake build instead of 10+. If for a particular build more files are listed than are present, the additional ones are just ignored.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] sl1pkn07 commented on a change in pull request
#17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
sl1pkn07 commented on a change in pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
URL: https://github.com/apache/incubator-mxnet/pull/17815#discussion_r394377382
##########
File path: CMakeLists.txt
##########
@@ -704,6 +712,8 @@ if(UNIX)
target_link_libraries(mxnet PRIVATE mxnet_static)
target_link_libraries(mxnet_static PUBLIC ${CMAKE_DL_LIBS})
set_target_properties(mxnet_static PROPERTIES OUTPUT_NAME mxnet)
+ set_target_properties(mxnet PROPERTIES VERSION "${ver_major}.${ver_minor}.${ver_patch}")
Review comment:
other test, and this surprise me, i just add
~~~cmake
message(STATUS "Project version: ${PROJECT_VERSION}")
~~~
just after `project()` function, and works
but is vanished if put that in the bottom/middle of the CMakeLists
cmake is a bit complex for me, IDK where is the problem, so, i think is better keep untounch with `${ver_major}.${ver_minor}.${ver_patch}` and in near future (when i found the problem), fix it
greetings
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] sl1pkn07 commented on a change in pull request
#17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
sl1pkn07 commented on a change in pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
URL: https://github.com/apache/incubator-mxnet/pull/17815#discussion_r394383299
##########
File path: CMakeLists.txt
##########
@@ -704,6 +712,8 @@ if(UNIX)
target_link_libraries(mxnet PRIVATE mxnet_static)
target_link_libraries(mxnet_static PUBLIC ${CMAKE_DL_LIBS})
set_target_properties(mxnet_static PROPERTIES OUTPUT_NAME mxnet)
+ set_target_properties(mxnet PROPERTIES VERSION "${ver_major}.${ver_minor}.${ver_patch}")
Review comment:
ook, i found the problem
`project()` is call twice in the main CMakeFile
https://github.com/apache/incubator-mxnet/blob/master/CMakeLists.txt#L9
https://github.com/apache/incubator-mxnet/blob/master/CMakeLists.txt#L89
why this?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] mseth10 commented on pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
mseth10 commented on pull request #17815:
URL: https://github.com/apache/incubator-mxnet/pull/17815#issuecomment-670771910
Seems like this commit breaks the CD pipeline, can you please take a look @sl1pkn07 @leezu
https://jenkins.mxnet-ci.amazon-ml.com/blue/organizations/jenkins/restricted-mxnet-cd%2Fmxnet-cd-release-job/detail/mxnet-cd-release-job/1521/pipeline/345
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] sl1pkn07 commented on a change in pull request
#17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
sl1pkn07 commented on a change in pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
URL: https://github.com/apache/incubator-mxnet/pull/17815#discussion_r394081119
##########
File path: CMakeLists.txt
##########
@@ -704,6 +712,8 @@ if(UNIX)
target_link_libraries(mxnet PRIVATE mxnet_static)
target_link_libraries(mxnet_static PUBLIC ${CMAKE_DL_LIBS})
set_target_properties(mxnet_static PROPERTIES OUTPUT_NAME mxnet)
+ set_target_properties(mxnet PROPERTIES VERSION "${ver_major}.${ver_minor}.${ver_patch}")
Review comment:
like this?
~~~cmake
set_target_properties(mxnet PROPERTIES VERSION "${PROJECT_VERSION}")
set_target_properties(mxnet PROPERTIES SOVERSION "${PROJECT_VERSION_MAJOR}")
~~~
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] sl1pkn07 commented on a change in pull request
#17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
sl1pkn07 commented on a change in pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
URL: https://github.com/apache/incubator-mxnet/pull/17815#discussion_r394043698
##########
File path: CMakeLists.txt
##########
@@ -1,5 +1,14 @@
cmake_minimum_required(VERSION 3.13)
+file(READ "include/mxnet/base.h" ver)
+string(REGEX MATCH "MXNET_MAJOR ([0-9]*)" _ ${ver})
+set(ver_major ${CMAKE_MATCH_1})
+string(REGEX MATCH "MXNET_MINOR ([0-9]*)" _ ${ver})
+set(ver_minor ${CMAKE_MATCH_1})
+string(REGEX MATCH "MXNET_PATCH ([0-9]*)" _ ${ver})
+set(ver_patch ${CMAKE_MATCH_1})
+set(_PROJECT_VERSION "${ver_major}.${ver_minor}.${ver_patch}")
Review comment:
because ikd can merge it, is is valid, then I merge it
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on a change in pull request
#17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
URL: https://github.com/apache/incubator-mxnet/pull/17815#discussion_r394069335
##########
File path: CMakeLists.txt
##########
@@ -704,6 +712,8 @@ if(UNIX)
target_link_libraries(mxnet PRIVATE mxnet_static)
target_link_libraries(mxnet_static PUBLIC ${CMAKE_DL_LIBS})
set_target_properties(mxnet_static PROPERTIES OUTPUT_NAME mxnet)
+ set_target_properties(mxnet PROPERTIES VERSION "${ver_major}.${ver_minor}.${ver_patch}")
Review comment:
https://cmake.org/cmake/help/latest/prop_tgt/VERSION.html does not say anything about the target property being initialized from the project version. So it should still be needed. But you can try it out locally: Don's set the target property but print it. I expect it would print an empty value.
Instead of using `ver_major` you can use https://cmake.org/cmake/help/latest/variable/PROJECT_VERSION_MAJOR.html#variable:PROJECT_VERSION_MAJOR etc
for clarity
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] sl1pkn07 commented on issue #17815: Add SOVERSION
when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
sl1pkn07 commented on issue #17815: Add SOVERSION when build shared libmxnet.so library
URL: https://github.com/apache/incubator-mxnet/pull/17815#issuecomment-597838839
Hi
That accepts things like 'libmxnet.so.*'?, Because the version depends on the value provided by 'include/mxnet.h'
Greetings
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #17815:
URL: https://github.com/apache/incubator-mxnet/pull/17815#issuecomment-665272530
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] sl1pkn07 commented on pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
sl1pkn07 commented on pull request #17815:
URL: https://github.com/apache/incubator-mxnet/pull/17815#issuecomment-671278295
the library is libmxnet.so.2.0.0. libmxnet.so and libmxnet.so.2 are symlinks of libmxnet.so.2.0.0
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] leezu commented on pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
leezu commented on pull request #17815:
URL: https://github.com/apache/incubator-mxnet/pull/17815#issuecomment-671626319
@mseth10 it's fine to include only a single file in the wheel
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mseth10 commented on pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
mseth10 commented on pull request #17815:
URL: https://github.com/apache/incubator-mxnet/pull/17815#issuecomment-671569611
@sl1pkn07 @leezu thanks for confirming that all three files need to be packaged in the nightly wheel. Will work on fixing the `artifact_repository.py` accordingly. What about @szha 's concern, do the md5 hashes make sense?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] sl1pkn07 commented on a change in pull request
#17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
sl1pkn07 commented on a change in pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
URL: https://github.com/apache/incubator-mxnet/pull/17815#discussion_r394362027
##########
File path: CMakeLists.txt
##########
@@ -704,6 +712,8 @@ if(UNIX)
target_link_libraries(mxnet PRIVATE mxnet_static)
target_link_libraries(mxnet_static PUBLIC ${CMAKE_DL_LIBS})
set_target_properties(mxnet_static PROPERTIES OUTPUT_NAME mxnet)
+ set_target_properties(mxnet PROPERTIES VERSION "${ver_major}.${ver_minor}.${ver_patch}")
Review comment:
without quotes
<details>
</details>
with quotes
<details>
</details>
if you see some examples you postes, `PROJECT_VERSION` is ser as sis before
https://github.com/fps/ladspa.m.plugins/blob/3d595382b72674e926fb52a426fa7689ce1aa999/CMakeLists.txt#L8
https://github.com/devcrmbmp812/QTwork/blob/56129534067aa2bc0875e538ce91f8e32ae84e72/CMakeLists.txt#L6
the only case i test and works is https://github.com/eyjohn/scratch-binary-compat/blob/master/CMakeLists.txt
but this, as you see in the captures, not works on mxnet
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] sl1pkn07 commented on a change in pull request
#17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
sl1pkn07 commented on a change in pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
URL: https://github.com/apache/incubator-mxnet/pull/17815#discussion_r394377382
##########
File path: CMakeLists.txt
##########
@@ -704,6 +712,8 @@ if(UNIX)
target_link_libraries(mxnet PRIVATE mxnet_static)
target_link_libraries(mxnet_static PUBLIC ${CMAKE_DL_LIBS})
set_target_properties(mxnet_static PROPERTIES OUTPUT_NAME mxnet)
+ set_target_properties(mxnet PROPERTIES VERSION "${ver_major}.${ver_minor}.${ver_patch}")
Review comment:
other test, ans this surprise me, i just add
~~~cmake
message(STATUS "Project version: ${PROJECT_VERSION}")
~~~
just before `project()` function, and works
but is vanished if put that in the bottom/middle of the CMakeLists
cmake is a bit complex for me, IDK where is the problem, so, i think is better keep untounch with `${ver_major}.${ver_minor}.${ver_patch}` and in near future (when i found the problem), fix it
greetings
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] sl1pkn07 commented on a change in pull request
#17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
sl1pkn07 commented on a change in pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
URL: https://github.com/apache/incubator-mxnet/pull/17815#discussion_r394362027
##########
File path: CMakeLists.txt
##########
@@ -704,6 +712,8 @@ if(UNIX)
target_link_libraries(mxnet PRIVATE mxnet_static)
target_link_libraries(mxnet_static PUBLIC ${CMAKE_DL_LIBS})
set_target_properties(mxnet_static PROPERTIES OUTPUT_NAME mxnet)
+ set_target_properties(mxnet PROPERTIES VERSION "${ver_major}.${ver_minor}.${ver_patch}")
Review comment:
without quotes
<details>
</details>
with quotes
<details>
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] sl1pkn07 commented on a change in pull request
#17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
sl1pkn07 commented on a change in pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
URL: https://github.com/apache/incubator-mxnet/pull/17815#discussion_r394377382
##########
File path: CMakeLists.txt
##########
@@ -704,6 +712,8 @@ if(UNIX)
target_link_libraries(mxnet PRIVATE mxnet_static)
target_link_libraries(mxnet_static PUBLIC ${CMAKE_DL_LIBS})
set_target_properties(mxnet_static PROPERTIES OUTPUT_NAME mxnet)
+ set_target_properties(mxnet PROPERTIES VERSION "${ver_major}.${ver_minor}.${ver_patch}")
Review comment:
other test, ans this surprise me, i just add
~~~cmake
message(STATUS "Project version: ${PROJECT_VERSION}")
~~~
just before `project()` function, and works
bus is vanished if put that in the bottom/middle of the CMakeLists
cmake is a bit complex for me, IDK where is the problem, so, i think is better keep untounch with `${ver_major}.${ver_minor}.${ver_patch}` and in near future (i i found the problem), fix it
greetings
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on issue #17815: Add SOVERSION
when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
leezu commented on issue #17815: Add SOVERSION when build shared libmxnet.so library
URL: https://github.com/apache/incubator-mxnet/pull/17815#issuecomment-597939587
Wildcards work. For example, `python/mxnet/_ffi/_cy3/*.so` is already used.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] szha commented on pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
szha commented on pull request #17815:
URL: https://github.com/apache/incubator-mxnet/pull/17815#issuecomment-671426654
I understand, which makes it weird that the libmxnet.so.2 and libmxnet.so.2.0.0 share the same md5 hash but libmxnet.so doesn't.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] szha commented on pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
szha commented on pull request #17815:
URL: https://github.com/apache/incubator-mxnet/pull/17815#issuecomment-671570612
@mseth10 my concern is on the CD pipeline, not on this PR. the problem has to be on the CD side for copying the artifacts.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] leezu commented on pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
leezu commented on pull request #17815:
URL: https://github.com/apache/incubator-mxnet/pull/17815#issuecomment-671519922
@mseth10 would you help to fix the `artifact_repository.py` and resubmit the PR as CD is currently not validated by the CI and contributors can't replicate the system.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mseth10 commented on a change in pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
mseth10 commented on a change in pull request #17815:
URL: https://github.com/apache/incubator-mxnet/pull/17815#discussion_r467711366
##########
File path: cd/mxnet_lib/static/Jenkins_pipeline.groovy
##########
@@ -26,7 +26,7 @@
// NOTE: the following variables are referenced in the mxnet_lib_pipeline jenkins file imported bellow
// libmxnet location
-libmxnet = 'lib/libmxnet.so'
+libmxnet = 'lib/libmxnet.so*'
Review comment:
With this change, are we planning to package all three binaries (viz. libmxnet.so, libmxnet.so.2, libmxnet.so.2.0.0) into the nightly wheel? @sl1pkn07 @leezu
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] leezu commented on pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
leezu commented on pull request #17815:
URL: https://github.com/apache/incubator-mxnet/pull/17815#issuecomment-671579664
Yes, `md5sum` would normally report the hash the file content that is being symlinked, and thus all hashes should be the same:
```
% ls -l test*
-rw-r--r-- 1 lausen domain^users 0 Aug 10 20:40 test
lrwxrwxrwx 1 lausen domain^users 4 Aug 10 20:41 test1 -> test
lrwxrwxrwx 1 lausen domain^users 5 Aug 10 20:41 test1.1 -> test1
% md5sum test*
d41d8cd98f00b204e9800998ecf8427e test
d41d8cd98f00b204e9800998ecf8427e test1
d41d8cd98f00b204e9800998ecf8427e test1.1
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] leezu commented on pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
leezu commented on pull request #17815:
URL: https://github.com/apache/incubator-mxnet/pull/17815#issuecomment-671627199
@mseth10 that's also a valid solution
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mseth10 commented on pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
mseth10 commented on pull request #17815:
URL: https://github.com/apache/incubator-mxnet/pull/17815#issuecomment-671614222
Reading about symlink support in wheels, there is no agreed upon solution in the community. There are some workarounds for unix-based platforms, but they dont work for windows. https://discuss.python.org/t/symbolic-links-in-wheels/1945
Would it make sense to copy just the `libmxnet.so` when building the wheels?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] szha commented on pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
szha commented on pull request #17815:
URL: https://github.com/apache/incubator-mxnet/pull/17815#issuecomment-671010491
The CD pipeline needs to be fixed. In addition, it looks like the CD pipeline is receiving libmxnet files with [varying file content](https://jenkins.mxnet-ci.amazon-ml.com/blue/organizations/jenkins/restricted-mxnet-cd%2Fmxnet-cd-release-job/detail/mxnet-cd-release-job/1524/pipeline/302#step-325-log-6)
```
[2020-08-08T20:16:46.129Z] + md5sum lib/libmxnet.so
[2020-08-08T20:16:46.129Z] da79e007aa331599ab5e474b9ade855d lib/libmxnet.so
[2020-08-08T20:16:46.129Z] + md5sum lib/libmxnet.so.2
[2020-08-08T20:16:46.689Z] 791b1014c70e5b421285a83f5fee63e7 lib/libmxnet.so.2
[2020-08-08T20:16:46.689Z] + md5sum lib/libmxnet.so.2.0.0
[2020-08-08T20:16:46.689Z] 791b1014c70e5b421285a83f5fee63e7 lib/libmxnet.so.2.0.0
```
I'm reverting this PR to unblock other development. That said, I totally understand that the CD pipeline is unnecessarily complex and it's a bit hard to track what caused the problem. @sandeep-krishnamurthy could your team take up reapplying the patch after revert and fix the CD for it?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] szha commented on pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
szha commented on pull request #17815:
URL: https://github.com/apache/incubator-mxnet/pull/17815#issuecomment-671582296
@mseth10 in fact we shouldn't need to copy all three files when building the wheel. the wrong md5 sum may indicate that some files are symlink or incomplete files.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] leezu removed a comment on pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
leezu removed a comment on pull request #17815:
URL: https://github.com/apache/incubator-mxnet/pull/17815#issuecomment-671626319
@mseth10 it's fine to include only a single file in the wheel
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] mseth10 commented on pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
mseth10 commented on pull request #17815:
URL: https://github.com/apache/incubator-mxnet/pull/17815#issuecomment-671577724
I'll look into it as well. So all three binary files must share the same md5 hash as they are just pointing to the same binary. Is my understanding correct?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] leezu merged pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
leezu merged pull request #17815:
URL: https://github.com/apache/incubator-mxnet/pull/17815
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-mxnet] leezu commented on pull request #17815: [WIP] Add SOVERSION when build shared libmxnet.so library
Posted by GitBox <gi...@apache.org>.
leezu commented on pull request #17815:
URL: https://github.com/apache/incubator-mxnet/pull/17815#issuecomment-665272450
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org