[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo with git
This revision was automatically updated to reflect the committed changes.
Closed by commit rL312865: [Basic] Update CMakeLists.txt to handle repo
(authored by MinSeongKIM).
Changed prior to commit:
https://reviews.llvm.org/D35533?vs=114160=114488#toc
Repository:
rL LLVM
https://reviews.llvm.org/D35533
Files:
cfe/trunk/lib/Basic/CMakeLists.txt
Index: cfe/trunk/lib/Basic/CMakeLists.txt
===
--- cfe/trunk/lib/Basic/CMakeLists.txt
+++ cfe/trunk/lib/Basic/CMakeLists.txt
@@ -4,39 +4,6 @@
Support
)
-# Figure out if we can track VC revisions.
-function(find_first_existing_file out_var)
- foreach(file ${ARGN})
-if(EXISTS "${file}")
- set(${out_var} "${file}" PARENT_SCOPE)
- return()
-endif()
- endforeach()
-endfunction()
-
-macro(find_first_existing_vc_file out_var path)
- set(git_path "${path}/.git")
-
- # Normally '.git' is a directory that contains a 'logs/HEAD' file that
- # is updated as modifications are made to the repository. In case the
- # repository is a Git submodule, '.git' is a file that contains text that
- # indicates where the repository's Git directory exists.
- if (EXISTS "${git_path}" AND NOT IS_DIRECTORY "${git_path}")
-FILE(READ "${git_path}" file_contents)
-if("${file_contents}" MATCHES "^gitdir: ([^\n]+)")
- # '.git' is indeed a link to the submodule's Git directory.
- # Use the path to that Git directory.
- set(git_path "${path}/${CMAKE_MATCH_1}")
-endif()
- endif()
-
- find_first_existing_file(${out_var}
-"${git_path}/logs/HEAD" # Git or Git submodule
-"${path}/.svn/wc.db" # SVN 1.7
-"${path}/.svn/entries" # SVN 1.6
-)
-endmacro()
-
find_first_existing_vc_file(llvm_vc "${LLVM_MAIN_SRC_DIR}")
find_first_existing_vc_file(clang_vc "${CLANG_SOURCE_DIR}")
Index: cfe/trunk/lib/Basic/CMakeLists.txt
===
--- cfe/trunk/lib/Basic/CMakeLists.txt
+++ cfe/trunk/lib/Basic/CMakeLists.txt
@@ -4,39 +4,6 @@
Support
)
-# Figure out if we can track VC revisions.
-function(find_first_existing_file out_var)
- foreach(file ${ARGN})
-if(EXISTS "${file}")
- set(${out_var} "${file}" PARENT_SCOPE)
- return()
-endif()
- endforeach()
-endfunction()
-
-macro(find_first_existing_vc_file out_var path)
- set(git_path "${path}/.git")
-
- # Normally '.git' is a directory that contains a 'logs/HEAD' file that
- # is updated as modifications are made to the repository. In case the
- # repository is a Git submodule, '.git' is a file that contains text that
- # indicates where the repository's Git directory exists.
- if (EXISTS "${git_path}" AND NOT IS_DIRECTORY "${git_path}")
-FILE(READ "${git_path}" file_contents)
-if("${file_contents}" MATCHES "^gitdir: ([^\n]+)")
- # '.git' is indeed a link to the submodule's Git directory.
- # Use the path to that Git directory.
- set(git_path "${path}/${CMAKE_MATCH_1}")
-endif()
- endif()
-
- find_first_existing_file(${out_var}
-"${git_path}/logs/HEAD" # Git or Git submodule
-"${path}/.svn/wc.db" # SVN 1.7
-"${path}/.svn/entries" # SVN 1.6
-)
-endmacro()
-
find_first_existing_vc_file(llvm_vc "${LLVM_MAIN_SRC_DIR}")
find_first_existing_vc_file(clang_vc "${CLANG_SOURCE_DIR}")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo with git
hintonda accepted this revision. hintonda added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D35533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo with git
minseong.kim updated this revision to Diff 114160.
minseong.kim marked 3 inline comments as done.
minseong.kim added a comment.
Re-uploading the patch, removing debug messages accidentally included in the
patch.
https://reviews.llvm.org/D35533
Files:
lib/Basic/CMakeLists.txt
Index: lib/Basic/CMakeLists.txt
===
--- lib/Basic/CMakeLists.txt
+++ lib/Basic/CMakeLists.txt
@@ -4,39 +4,6 @@
Support
)
-# Figure out if we can track VC revisions.
-function(find_first_existing_file out_var)
- foreach(file ${ARGN})
-if(EXISTS "${file}")
- set(${out_var} "${file}" PARENT_SCOPE)
- return()
-endif()
- endforeach()
-endfunction()
-
-macro(find_first_existing_vc_file out_var path)
- set(git_path "${path}/.git")
-
- # Normally '.git' is a directory that contains a 'logs/HEAD' file that
- # is updated as modifications are made to the repository. In case the
- # repository is a Git submodule, '.git' is a file that contains text that
- # indicates where the repository's Git directory exists.
- if (EXISTS "${git_path}" AND NOT IS_DIRECTORY "${git_path}")
-FILE(READ "${git_path}" file_contents)
-if("${file_contents}" MATCHES "^gitdir: ([^\n]+)")
- # '.git' is indeed a link to the submodule's Git directory.
- # Use the path to that Git directory.
- set(git_path "${path}/${CMAKE_MATCH_1}")
-endif()
- endif()
-
- find_first_existing_file(${out_var}
-"${git_path}/logs/HEAD" # Git or Git submodule
-"${path}/.svn/wc.db" # SVN 1.7
-"${path}/.svn/entries" # SVN 1.6
-)
-endmacro()
-
find_first_existing_vc_file(llvm_vc "${LLVM_MAIN_SRC_DIR}")
find_first_existing_vc_file(clang_vc "${CLANG_SOURCE_DIR}")
Index: lib/Basic/CMakeLists.txt
===
--- lib/Basic/CMakeLists.txt
+++ lib/Basic/CMakeLists.txt
@@ -4,39 +4,6 @@
Support
)
-# Figure out if we can track VC revisions.
-function(find_first_existing_file out_var)
- foreach(file ${ARGN})
-if(EXISTS "${file}")
- set(${out_var} "${file}" PARENT_SCOPE)
- return()
-endif()
- endforeach()
-endfunction()
-
-macro(find_first_existing_vc_file out_var path)
- set(git_path "${path}/.git")
-
- # Normally '.git' is a directory that contains a 'logs/HEAD' file that
- # is updated as modifications are made to the repository. In case the
- # repository is a Git submodule, '.git' is a file that contains text that
- # indicates where the repository's Git directory exists.
- if (EXISTS "${git_path}" AND NOT IS_DIRECTORY "${git_path}")
-FILE(READ "${git_path}" file_contents)
-if("${file_contents}" MATCHES "^gitdir: ([^\n]+)")
- # '.git' is indeed a link to the submodule's Git directory.
- # Use the path to that Git directory.
- set(git_path "${path}/${CMAKE_MATCH_1}")
-endif()
- endif()
-
- find_first_existing_file(${out_var}
-"${git_path}/logs/HEAD" # Git or Git submodule
-"${path}/.svn/wc.db" # SVN 1.7
-"${path}/.svn/entries" # SVN 1.6
-)
-endmacro()
-
find_first_existing_vc_file(llvm_vc "${LLVM_MAIN_SRC_DIR}")
find_first_existing_vc_file(clang_vc "${CLANG_SOURCE_DIR}")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo with git
minseong.kim updated this revision to Diff 114115.
minseong.kim edited the summary of this revision.
minseong.kim added a reviewer: hintonda.
minseong.kim removed a subscriber: hintonda.
minseong.kim added a comment.
I have updated the diff.
https://reviews.llvm.org/D35533
Files:
lib/Basic/CMakeLists.txt
Index: lib/Basic/CMakeLists.txt
===
--- lib/Basic/CMakeLists.txt
+++ lib/Basic/CMakeLists.txt
@@ -4,40 +4,6 @@
Support
)
-# Figure out if we can track VC revisions.
-function(find_first_existing_file out_var)
- foreach(file ${ARGN})
-if(EXISTS "${file}")
- set(${out_var} "${file}" PARENT_SCOPE)
- return()
-endif()
- endforeach()
-endfunction()
-
-macro(find_first_existing_vc_file out_var path)
- set(git_path "${path}/.git")
-
- # Normally '.git' is a directory that contains a 'logs/HEAD' file that
- # is updated as modifications are made to the repository. In case the
- # repository is a Git submodule, '.git' is a file that contains text that
- # indicates where the repository's Git directory exists.
- if (EXISTS "${git_path}" AND NOT IS_DIRECTORY "${git_path}")
-FILE(READ "${git_path}" file_contents)
-if("${file_contents}" MATCHES "^gitdir: ([^\n]+)")
- # '.git' is indeed a link to the submodule's Git directory.
- # Use the path to that Git directory.
- set(git_path "${path}/${CMAKE_MATCH_1}")
-endif()
- endif()
-
- find_first_existing_file(${out_var}
-"${git_path}/logs/HEAD" # Git or Git submodule
-"${path}/.svn/wc.db" # SVN 1.7
-"${path}/.svn/entries" # SVN 1.6
-"${git_path}/HEAD" # Repo
-)
-endmacro()
-
find_first_existing_vc_file(llvm_vc "${LLVM_MAIN_SRC_DIR}")
find_first_existing_vc_file(clang_vc "${CLANG_SOURCE_DIR}")
Index: lib/Basic/CMakeLists.txt
===
--- lib/Basic/CMakeLists.txt
+++ lib/Basic/CMakeLists.txt
@@ -4,40 +4,6 @@
Support
)
-# Figure out if we can track VC revisions.
-function(find_first_existing_file out_var)
- foreach(file ${ARGN})
-if(EXISTS "${file}")
- set(${out_var} "${file}" PARENT_SCOPE)
- return()
-endif()
- endforeach()
-endfunction()
-
-macro(find_first_existing_vc_file out_var path)
- set(git_path "${path}/.git")
-
- # Normally '.git' is a directory that contains a 'logs/HEAD' file that
- # is updated as modifications are made to the repository. In case the
- # repository is a Git submodule, '.git' is a file that contains text that
- # indicates where the repository's Git directory exists.
- if (EXISTS "${git_path}" AND NOT IS_DIRECTORY "${git_path}")
-FILE(READ "${git_path}" file_contents)
-if("${file_contents}" MATCHES "^gitdir: ([^\n]+)")
- # '.git' is indeed a link to the submodule's Git directory.
- # Use the path to that Git directory.
- set(git_path "${path}/${CMAKE_MATCH_1}")
-endif()
- endif()
-
- find_first_existing_file(${out_var}
-"${git_path}/logs/HEAD" # Git or Git submodule
-"${path}/.svn/wc.db" # SVN 1.7
-"${path}/.svn/entries" # SVN 1.6
-"${git_path}/HEAD" # Repo
-)
-endmacro()
-
find_first_existing_vc_file(llvm_vc "${LLVM_MAIN_SRC_DIR}")
find_first_existing_vc_file(clang_vc "${CLANG_SOURCE_DIR}")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo with git
hintonda added a comment. In https://reviews.llvm.org/D35533#862852, @minseong.kim wrote: > @hintonda, Absolutely. Incorporating @modocache's module changes into the > version in AddLLVM.cmake would solve the current version display issue for > repo and do not affect the process of other version control systems (e.g. > git, git-svn, svn, and git submodule). I think we crossed, but yes, the version in AddLLVM.cmake handles everything correctly. So, I'd recommend removing these versions. https://reviews.llvm.org/D35533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo with git
minseong.kim added a comment. @hintonda, Absolutely. Incorporating @modocache's module changes into the version in AddLLVM.cmake would solve the current version display issue for repo and do not affect the process of other version control systems (e.g. git, git-svn, svn, and git submodule). https://reviews.llvm.org/D35533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo with git
hintonda added a comment. I just looked into both approaches and believe the one taken in AddLLVM.cmake is correct and handles the submodule case correctly. It uses `git rev-parse --git-dir` to find the location of the actual .git directory. Please see https://reviews.llvm.org/D31985 for details. https://reviews.llvm.org/D35533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo with git
hintonda added a comment. In https://reviews.llvm.org/D35533#862798, @minseong.kim wrote: > Hi~ @hintonda, > > Using using find_first_existing_file in ADDLLVM.cmake solves the cases with > repo in conjunction with https://reviews.llvm.org/D35532. However, I am not > sure it can handle @modocache's git submodule cases > (https://reviews.llvm.org/D34955). > > @modocache, could you please check removing find_first_existing_file and > find_first_existing_vc_file from lib/Basic/CMakeLists.txt solves git > submodule cases ? > > If so, I will remove them from clang's cmake to make one unified place for > the same functionality. Essentially, I think having a single version that works in all cases would be best. I really haven't looked into it, but would it make sense to incorporate @modocache's module changes into the version in AddLLVM.cmake? https://reviews.llvm.org/D35533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo with git
minseong.kim added a comment. Hi~ @hintonda, Using using find_first_existing_file in ADDLLVM.cmake solves the cases with repo in conjunction with https://reviews.llvm.org/D35532. However, I am not sure it can handle @modocache's git submodule cases (https://reviews.llvm.org/D34955). @modocache, could you please check removing find_first_existing_file and find_first_existing_vc_file from lib/Basic/CMakeLists.txt solves git submodule cases ? If so, I will remove them from clang's cmake to make one unified place for the same functionality. https://reviews.llvm.org/D35533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo with git
hintonda added a comment. Now that 36971 has been committed, I believe you can just remove the `find_first_existing_file` and `find_first_existing_vc_file` macro definitions from this file. https://reviews.llvm.org/D35533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo with git
minseong.kim added a comment. I have updated the description with a hope for it to be more descriptive. Kindly ping~ https://reviews.llvm.org/D35533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo
minseong.kim added a comment. I will test your patch with repo. Thanks for your time and efforts, @hintonda. https://reviews.llvm.org/D35533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo
hintonda added a comment.
I've submitted a patch, https://reviews.llvm.org/D36971, that moves
find_first_existing_file and
find_first_existing_vc_file to ADDLLVM so they can be reused here. If that
patch is accepted and lands,
you can then remove these versions and check to see if that solves your problem.
Note that you must remove these versions in order for cmake to use the moved
ones, which would be hidden
by the new ones, but can still be accessed by adding a '_' prefix.
Comment at: lib/Basic/CMakeLists.txt:17
macro(find_first_existing_vc_file out_var path)
set(git_path "${path}/.git")
minseong.kim wrote:
> hintonda wrote:
> > LLVM already has a version of find_first_existing_vc_file in
> > llvm/include/llvm/Support/CMakelists.txt.
> >
> > Would it make sense move it to an llvm module and reuse it in clang?
> Thanks for the suggestion.
> My understanding is that "llvm/include/llvm/Support/CMakeLists.txt" is used
> to generate VCSRevision.h which is used by llvm-specific modules such as opt,
> not clang. Furthermore find_first_existing_vc_file function in
> llvm/include/llvm/Support/CMakeLists.txt does not handle the version info
> either.
I believe the version of `find_first_vc_file` in llvm/Support/CMakeLists.txt
handles this case correctly. So, instead of fixing/maintaining this version,
which is slightly different, perhaps the version in llvm/Support/CMakeList.txt
could be moved to ADDLLVM.cmake where it could be used by both.
https://reviews.llvm.org/D35533
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo
minseong.kim added a comment. kindly ping~ https://reviews.llvm.org/D35533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo
minseong.kim added a comment. Thanks @jordan_rose @modocache @hintonda for your time and efforts. This patch does regenerate the version control info correctly (SVNVersion.inc) every time I re-make clang. Probably I am missing something here. Could you please be more specific and share your idea about "adding all of these as dependencies" to force reconfigure revision info whenever new commits added. I will gladly change this patch accordingly. https://reviews.llvm.org/D35533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo
jordan_rose added a subscriber: modocache.
jordan_rose added a comment.
> The addition of "${path}/.git/HEAD" at line 37 is used only once when repo is
> initially synced "for the first time". This is because ${git_path}/logs/HEAD
> file at line 34 for Git or Git submodule does not exist with initial repo
> sync[.]
Hm, interesting. Unfortunately, adding more commits won't automatically force a
reconfigure. Maybe we should consider adding //all// of these as dependencies?
cc @modocache, who just changed some of this.
https://reviews.llvm.org/D35533
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo
minseong.kim added a comment.
Thanks for the reviewing this patch. Please correct if I am wrong.
The addition of "${path}/.git/HEAD" at line 37 is used only once when repo is
initially synced "for the first time". This is because ${git_path}/logs/HEAD
file at line 34 for Git or Git submodule does not exist with initial repo sync
and therefore no generation of SVNVersion.inc which is referenced by --version
option, resulting null for the revision values of clang and llvm. After the
second repo sync, the first file "${git_path}/logs/HEAD" will be created and
can be used to generate "SVNVersion.inc". This enables correct version
information to be displayed even if new changes are merged.
Comment at: lib/Basic/CMakeLists.txt:17
macro(find_first_existing_vc_file out_var path)
set(git_path "${path}/.git")
hintonda wrote:
> LLVM already has a version of find_first_existing_vc_file in
> llvm/include/llvm/Support/CMakelists.txt.
>
> Would it make sense move it to an llvm module and reuse it in clang?
Thanks for the suggestion.
My understanding is that "llvm/include/llvm/Support/CMakeLists.txt" is used to
generate VCSRevision.h which is used by llvm-specific modules such as opt, not
clang. Furthermore find_first_existing_vc_file function in
llvm/include/llvm/Support/CMakeLists.txt does not handle the version info
either.
https://reviews.llvm.org/D35533
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo
hintonda added inline comments.
Comment at: lib/Basic/CMakeLists.txt:17
macro(find_first_existing_vc_file out_var path)
set(git_path "${path}/.git")
LLVM already has a version of find_first_existing_vc_file in
llvm/include/llvm/Support/CMakelists.txt.
Would it make sense move it to an llvm module and reuse it in clang?
https://reviews.llvm.org/D35533
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo
jordan_rose added a comment. As commented at https://reviews.llvm.org/D34955#798369, this isn't sufficient because it doesn't force a rebuild when new changes are merged in from upstream. It's not //harmful// to stick with the old version information, but if you can find something better that would be, um, better. https://reviews.llvm.org/D35533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo
minseong.kim created this revision.
Herald added a subscriber: mgorny.
When repo is used, '--version' option does not display correct version
information (i.e. Git hashes). This change makes the parsing of the
version info correctly recognise svn, git, git-svn and repo. This
in turn enables the option displays correct version information with
repo. Tested with git, svn and repo with this patch.
https://reviews.llvm.org/D35533
Files:
lib/Basic/CMakeLists.txt
Index: lib/Basic/CMakeLists.txt
===
--- lib/Basic/CMakeLists.txt
+++ lib/Basic/CMakeLists.txt
@@ -34,6 +34,7 @@
"${git_path}/logs/HEAD" # Git or Git submodule
"${path}/.svn/wc.db" # SVN 1.7
"${path}/.svn/entries" # SVN 1.6
+"${path}/.git/HEAD" # Repo
)
endmacro()
Index: lib/Basic/CMakeLists.txt
===
--- lib/Basic/CMakeLists.txt
+++ lib/Basic/CMakeLists.txt
@@ -34,6 +34,7 @@
"${git_path}/logs/HEAD" # Git or Git submodule
"${path}/.svn/wc.db" # SVN 1.7
"${path}/.svn/entries" # SVN 1.6
+"${path}/.git/HEAD" # Repo
)
endmacro()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
