|
Notes |
|
|
(0021943)
|
|
David Cole
|
|
2010-08-25 07:37
|
|
|
We cannot include this patch as is in CMake because it has a GPL license clause in it. |
|
|
|
(0021944)
|
|
David Cole
|
|
2010-08-25 07:39
|
|
|
Do not want GPL-ed code in CMake. |
|
|
|
(0021954)
|
|
Marcel Loose
|
|
2010-08-25 10:34
|
|
I have changed the license to BSD, referring to COPYING-CMAKE-SCRIPTS file.
I'll upload a new patch. |
|
|
|
(0022098)
|
|
David Cole
|
|
2010-09-06 11:58
|
|
Since you've changed to the BSD license, would you mind if this macro simply gets added to the existing FindSubversion.cmake module?
Would you be interested in becoming the FindSubversion.cmake module maintainer? |
|
|
|
(0022100)
|
|
Marcel Loose
|
|
2010-09-07 03:12
|
|
|
Yeah, that's OK. I don't think it will take too much of my time. Should I take any action, or is that something that you, David, need to do? |
|
|
|
(0022110)
|
|
David Cole
|
|
2010-09-07 13:43
|
|
|
|
|
(0022113)
|
|
David Cole
|
|
2010-09-07 15:18
|
|
|
|
|
(0022219)
|
|
David Cole
|
|
2010-09-14 09:19
|
|
|
We will not be addressing this for the upcoming CMake 2.8.3 release. Changing target release value back to empty for now. |
|
|
|
(0022223)
|
|
Marcel Loose
|
|
2010-09-15 04:28
|
|
|
Added Subversion_WC_UPDATE macro. |
|
|
|
(0022239)
|
|
David Cole
|
|
2010-09-15 17:59
|
|
|
There is no commit in 'next' on the CMake topic stage to resolve this issue. Please push commits to 'next' using the CMake topic stage before resolving issues. Thanks! |
|
|
|
(0022272)
|
|
Marcel Loose
|
|
2010-09-20 04:44
|
|
|
Should be OK now. I checked with gitweb. |
|
|
|
(0023128)
|
|
David Cole
|
|
2010-11-10 07:40
|
|
Hi Marcel,
Brad and I were reviewing this issue yesterday, looking at merging it to 'master', and I have a few questions about this macro.
Why make it take a list of directory names?
And why do the inner loop over the subdirectories, calling 'svn up -N' on each?
It seems to me that a simpler macro, that takes a single directory and just does an 'svn up' on the one directory given, would be just as useful. (And the code would be much easier to read and understand.)
What is the point of the inner loop and the 'svn up -N' calls?
Also, my "svn help update" docs say about -N : "obsolete; try --depth=files or --depth=immediates"
Before we merge this to 'master', would you mind answering these questions?
Thanks,
David |
|
|
|
(0024234)
|
|
David Cole
|
|
2010-12-17 11:24
|
|
|
This is still a work in progress... still pending on the stage. Changing from 'resolved' to 'feedback' until the work is done. |
|
|
|
(0024270)
|
|
Marcel Loose
|
|
2010-12-20 05:15
|
|
For completeness, I'll add the relevant parts of the email conversation with David.
> Why make it take a list of directory names?
Why not? That way the user can simply supply a list of directories he wants to update, instead of having to loop over that list himself. It yields cleaner CMakeLists.txt files which IMHO is preferred.
> And why do the inner loop over the subdirectories, calling 'svn up -N' on each?
That has to do with SVN's illogical behavior. If you do an svn update on a directory for which its parent is not yet part of the working copy, then svn will silently fail printing something like '<directory> skipped' and return with exit status 0.
> Also, my "svn help update" docs say about -N : "obsolete; try --depth=files or --depth=immediates"
Yes I know. Problem is that we're still using svn 1.4 and these new
options were introduced in version 1.5. My guess is that -N will be
supported quite some time to be backward compatible and avoid breaking
existing code. |
|
|
|
(0024271)
|
|
Marcel Loose
|
|
2010-12-20 05:17
|
|
I ran into an interesting issue while trying to add some tests for Subversion_WC_UPDATE.
I use get_filename_component() to the the absolute path of the specified directories. I then strip off CMAKE_SOURCE_DIR. Turns out, though, that CMAKE_SOURCE_DIR is a REALPATH, not just an ABSOLUTE path. So, this fails whenever your absolute path contains a symlink. Should I use REALPATH instead in Subversion_WC_UPDATE to get the full path? |
|
|
|
(0024272)
|
|
Marcel Loose
|
|
2010-12-20 05:19
|
|
David Cole wrote:
Glad you ran into that. This is one of the concerns I had about this
code: I did not understand why it was doing anything with respect to
CMAKE_SOURCE_DIR anyhow... It's a macro intended to run "svn up" in a
given source directory. Why should CMAKE_SOURCE_DIR necessarily be
related to the directory that a user passes in that he wants to run
"svn up" in?
I would say: you shouldn't be stripping off CMAKE_SOURCE_DIR anyway,
so the question about REALPATH/ABSOLUTE is irrelevant. Make the code
work with the directory passed in, not some directory that you
compute.
What if I want to run this macro on a checkout that I have, whose root
svn directory is in a subdir of CMAKE_BINARY_DIR? (Quite a common case
for people using the ExternalProject_Add function...) |
|
|
|
(0024273)
|
|
Marcel Loose
|
|
2010-12-20 05:22
|
|
Marcel Loose wrote:
The reason I was converting to an absolute path is that I wanted the user to be able to use relative paths. Maybe that is already a bad idea in the first place. OTOH, I've run into problems before with CMake; sometimes it uses the absolute path, sometimes it uses the real path. See issue 0010335.
But, there's also a problematic issue with svn itself. In order to 'update' a working copy with a not-already checked out directory, you need to 'svn update -N' all directories between the top-level (repository root) directory and the given directory. For example, suppose someone did the following:
$ svn co -N <url-to-repos> <wd>
Next, he wants to 'update' <wd>/foo/bar; a directory that exists in the repository, but not yet in his working copy. 'svn up <wd>/foo/bar' won't work, because <wd>/foo does not yet exist. The worst of it all is that this command will fail silently. The right way to do this is:
$ svn up -N <wd>/foo
$ svn up <wd>/foo/bar
That's the reason I convert paths to absolute and strip off CMAKE_SOURCE_DIR. I hereby assume that CMAKE_SOURCE_DIR is also the repository-root directory. Maybe this is too fragile, but I don't know another way to do it. I'm open for suggestions. |
|
|
|
(0024274)
|
|
Marcel Loose
|
|
2010-12-20 05:23
|
|
David Cole wrote:
I, for one, would not assume that this macro would work, *except* on a
working copy of an svn checkout.
If you need to strip off a "root dir" of some sort, then the root dir
should be passed in as an argument to the macro. In which case, I
would argue for making two macros: one that does the simple 'svn up'
that I'm expecting (that takes just one dir as an argument), and the
other doing the fancy thing that you're trying to accomodate (which I
can't say I've ever actually even run into...) -- in the fancy
version, you'll have to pass in the root dir as an arg, because you
can't assume that source trees are underneath "CMAKE_SOURCE_DIR". |
|
|
|
(0024275)
|
|
Rolf Eike Beer
|
|
2010-12-20 05:36
|
|
|
I second that. This macro should just update the directory (or directories if the list form will be supported) and that's it. If a user needs to do additional tricks like digging deeper into a hierarchy he should do that using his own magic. That way we could also get rid of the -N. If the user wants directory expansion he should do that himself. Otherwise he will just get an update of what he already has. |
|
|
|
(0024397)
|
|
David Cole
|
|
2011-01-04 12:01
|
|
|
Not enough time to finalize this work and merge it in for CMake 2.8.4 -- unsetting target version field -- bring back for a future roadmap later. |
|
|
|
(0041648)
|
|
Kitware Robot
|
|
2016-06-10 14:27
|
|
Resolving issue as `moved`.
This issue tracker is no longer used. Further discussion of this issue may take place in the current CMake Issues page linked in the banner at the top of this page. |
|
Subversion_WC_UPDATE.cmake (2,577) 2010-01-29 05:25