Skip to content
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

cmake: sync nvml submodule to latest code. #20411

Merged
merged 2 commits into from Feb 28, 2018

Conversation

Projects
None yet
3 participants
@majianpeng
Copy link
Member

commented Feb 13, 2018

Signed-off-by: Jianpeng Ma jianpeng.ma@intel.com

@liewegas liewegas added the build/ops label Feb 13, 2018

@tchaikov

This comment has been minimized.

Copy link
Contributor

commented Feb 13, 2018

@majianpeng probably we can just use the sha1 of the upstream ? i.e. pmem/pmdk@cdca620 . what do you think? if you are good with it. i will force push it to ceph/nvml .

@majianpeng

This comment has been minimized.

Copy link
Member Author

commented Feb 13, 2018

@tchaikov . If use this method, is it not need ceph/nvml?

@tchaikov

This comment has been minimized.

Copy link
Contributor

commented Feb 13, 2018

@majianpeng i guess the point of using ceph/nvml is to ensure that we are able to stash not-yet-upstreamed fix in our fork. and yes. we can always point the GIT_REPOSITORY ceph/nvml in that case. i don't feel strong either way.

@majianpeng

This comment has been minimized.

Copy link
Member Author

commented Feb 14, 2018

@tchaikov . sorry to late response. I'll modify by your suggestion.

@majianpeng

This comment has been minimized.

Copy link
Member Author

commented Feb 14, 2018

@tchaikov . From the man git-clone. It only clone on a tag rather than a random commit. For the sha1 of the upstream ? i.e. pmem/pmdk@cdca620, it don't work. Can you give me a example? Thanks!

@tchaikov

This comment has been minimized.

Copy link
Contributor

commented Feb 14, 2018

@majianpeng what i had in my mind was:

diff --git a/src/os/CMakeLists.txt b/src/os/CMakeLists.txt
index 32a58e402e..b773e9329d 100644
--- a/src/os/CMakeLists.txt
+++ b/src/os/CMakeLists.txt
@@ -123,7 +123,7 @@ if(WITH_PMEM)
   ExternalProject_Add(nvml_ext
     DOWNLOAD_DIR ${CMAKE_BINARY_DIR}/src/
     GIT_REPOSITORY "https://github.com/ceph/nvml.git"
-    GIT_TAG "dd5b62dbc2cbbe048087dea"
+    GIT_TAG "cdca620c361714aef40f051"
     SOURCE_DIR ${CMAKE_BINARY_DIR}/src/nvml
     CONFIGURE_COMMAND ""
     BUILD_COMMAND $(MAKE)

after "git push -f" cdca620c361714aef40f051 to ceph/nvml.
but it seems neither do i have the write access to https://github.com/ceph/nvml.git . @liewegas could you grant me the write access to that repo as well?

@liewegas

This comment has been minimized.

Copy link
Member

commented Feb 15, 2018

@tchaikov should work now?

@tchaikov

This comment has been minimized.

Copy link
Contributor

commented Feb 16, 2018

@liewegas thanks. it works now.

@majianpeng could you update this PR? as https://github.com/ceph/nvml is sync'ed with its upstream now.

@tchaikov

This comment has been minimized.

Copy link
Contributor

commented Feb 16, 2018

retest this please.

@majianpeng majianpeng force-pushed the majianpeng:sync-nvml-submodule branch from 21fb587 to 5c38b88 Feb 27, 2018

@majianpeng

This comment has been minimized.

Copy link
Member Author

commented Feb 27, 2018

@tchaikov . sorry for later response. I still had a question: this can reduce sync ceph/nvml operation. etc, if pmdk HEAD tag is abcde. If the tag change to abcde. After this PR merged, ceph/nvml update to abcde. Or am i missing something? Hope your explain. Thanks very much!

@tchaikov

This comment has been minimized.

Copy link
Contributor

commented Feb 27, 2018

if pmdk HEAD tag is abcde. If the tag change to abcde. After this PR merged, ceph/nvml update to abcde.

no. ceph/nvml is already sync'ed with upstream by me. that's why i asked for the write access from Sage. and yes, you can point the GIT_REPOSITORY directly to the upstream, and point it back to ceph/nvml if we want to include a change which is stashed in our fork, and it is not yet merged by upstream.

@majianpeng majianpeng force-pushed the majianpeng:sync-nvml-submodule branch from 5c38b88 to f85674a Feb 28, 2018

@tchaikov tchaikov merged commit d0dd6e8 into ceph:master Feb 28, 2018

5 checks passed

Docs: build check OK - docs built
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details
make check (arm64) make check succeeded
Details

majianpeng added some commits Feb 27, 2018

cmake: sync nvml submodule
Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
os/bluestore: make PMEMDevice work based on latest pmdk.
Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.