You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "kou (via GitHub)" <gi...@apache.org> on 2023/10/04 01:45:51 UTC
[PR] GH-37767: [C++][CMake] Don't touch .git/index [arrow]
kou opened a new pull request, #38003:
URL: https://github.com/apache/arrow/pull/38003
### Rationale for this change
We run "git describe --tag --dirty" implicitly in
cpp/cmake_modules/DefineOptions.cmake. If we use "--dirty", .git/index's owner may be changed. Because "git describe" touches .git/index for "--dirty".
We can avoid changing .git/index's owner by not using "--dirty".
### What changes are included in this PR?
Remove "--dirty".
### Are these changes tested?
Yes. I used "strace git describe ...".
### Are there any user-facing changes?
No.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37767: [C++][CMake] Don't touch .git/index [arrow]
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser merged PR #38003:
URL: https://github.com/apache/arrow/pull/38003
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37767: [C++][CMake] Don't touch .git/index [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #38003:
URL: https://github.com/apache/arrow/pull/38003#discussion_r1350418718
##########
cpp/cmake_modules/DefineOptions.cmake:
##########
@@ -747,7 +747,7 @@ if(NOT ARROW_GIT_ID)
OUTPUT_STRIP_TRAILING_WHITESPACE)
endif()
if(NOT ARROW_GIT_DESCRIPTION)
- execute_process(COMMAND "git" "describe" "--tags" "--dirty"
+ execute_process(COMMAND "git" "describe" "--tags"
Review Comment:
Are you sure this fixes the issue reliably? Is it just an implementation detail in git?
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37767: [C++][CMake] Don't touch .git/index [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #38003:
URL: https://github.com/apache/arrow/pull/38003#issuecomment-1761628803
After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 277b168a95f32cd716fba29956784ed5ac2e977d.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/17681477883) has more details. It also includes information about 1 possible false positive for unstable benchmarks that are known to sometimes produce them.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37767: [C++][CMake] Don't touch .git/index [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #38003:
URL: https://github.com/apache/arrow/pull/38003#issuecomment-1746001397
:warning: GitHub issue #37767 **has been automatically assigned in GitHub** to PR creator.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37767: [C++][CMake] Don't touch .git/index [arrow]
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on code in PR #38003:
URL: https://github.com/apache/arrow/pull/38003#discussion_r1353821127
##########
cpp/cmake_modules/DefineOptions.cmake:
##########
@@ -747,7 +747,7 @@ if(NOT ARROW_GIT_ID)
OUTPUT_STRIP_TRAILING_WHITESPACE)
endif()
if(NOT ARROW_GIT_DESCRIPTION)
- execute_process(COMMAND "git" "describe" "--tags" "--dirty"
+ execute_process(COMMAND "git" "describe" "--tags"
Review Comment:
wow great investigation!
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37767: [C++][CMake] Don't touch .git/index [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #38003:
URL: https://github.com/apache/arrow/pull/38003#discussion_r1350790230
##########
cpp/cmake_modules/DefineOptions.cmake:
##########
@@ -747,7 +747,7 @@ if(NOT ARROW_GIT_ID)
OUTPUT_STRIP_TRAILING_WHITESPACE)
endif()
if(NOT ARROW_GIT_DESCRIPTION)
- execute_process(COMMAND "git" "describe" "--tags" "--dirty"
+ execute_process(COMMAND "git" "describe" "--tags"
Review Comment:
It's an implementation detail.
The `git describe` document doesn't explain this behavior: https://git-scm.com/docs/git-describe
Here is the implementation with `--dirty`:
https://github.com/git/git/blob/3a06386e314565108ad56a9bdb8f7b80ac52fb69/builtin/describe.c#L669-L697
It touches `.git/index` (`*_index()` functions are 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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org