(0021943)
David Cole (manager)
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 (manager)
2010-08-25 07:39

Do not want GPL-ed code in CMake.

(0021954)
Marcel Loose (developer)
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 (manager)
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 (developer)
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 (manager)
2010-09-07 13:43

To become the FindSubversion.cmake module maintainer, simply follow the directions here:
http://www.cmake.org/Wiki/CMake:Module_Maintainers [^]

To request git privileges, send us your public key as outlined here:
http://www.cmake.org/Wiki/CMake/Git#Publishing [^]
http://www.cmake.org/Wiki/CMake/Git#Topic_Stage [^]

Send mail to the list saying that you are volunteering to become the maintainer, send us the public side of your ssh key so that we can give you git push privileges, and then you can fix this bug by adding to FindSubversion.cmake, committing and pushing to 'next'. After you do that, and we've had a green dashboard overnight, Brad and I will merge the changes to master for the following release.

Thanks!

(0022113)
David Cole (manager)
2010-09-07 15:18

Hi Marcel,

In addition to the instructions in the previous note, ... to be a CMake module maintainer, you should also be subscribed to the CMake developers mailing list.

Sign up for that here:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers [^]

Thanks again.

(0022219)
David Cole (manager)
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 (developer)
2010-09-15 04:28

Added Subversion_WC_UPDATE macro.

(0022239)
David Cole (manager)
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 (developer)
2010-09-20 04:44

Should be OK now. I checked with gitweb.

(0023128)
David Cole (manager)
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 (manager)
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 (developer)
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 (developer)
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 (developer)
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 (developer)
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 (developer)
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 (developer)
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 (manager)
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 (administrator)
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.

Issue History
Date Modified	Username	Field	Change
2010-01-29 05:25	Marcel Loose	New Issue
2010-01-29 05:25	Marcel Loose	File Added: Subversion_WC_UPDATE.cmake
2010-08-25 07:37	David Cole	Note Added: 0021943
2010-08-25 07:37	David Cole	Status	new => assigned
2010-08-25 07:37	David Cole	Assigned To	=> David Cole
2010-08-25 07:39	David Cole	Note Added: 0021944
2010-08-25 07:39	David Cole	Status	assigned => resolved
2010-08-25 07:39	David Cole	Resolution	open => won't fix
2010-08-25 10:34	Marcel Loose	Note Added: 0021954
2010-08-25 10:34	Marcel Loose	Status	resolved => feedback
2010-08-25 10:34	Marcel Loose	Resolution	won't fix => reopened
2010-08-25 10:36	Marcel Loose	File Added: Subversion_WC_UPDATE-BSD-license.cmake
2010-08-31 18:10	David Cole	Target Version	=> CMake 2.8.3
2010-09-06 11:58	David Cole	Note Added: 0022098
2010-09-07 03:12	Marcel Loose	Note Added: 0022100
2010-09-07 13:43	David Cole	Note Added: 0022110
2010-09-07 15:18	David Cole	Note Added: 0022113
2010-09-08 12:07	David Cole	Status	feedback => assigned
2010-09-08 12:07	David Cole	Assigned To	David Cole => Marcel Loose
2010-09-14 09:19	David Cole	Note Added: 0022219
2010-09-14 09:19	David Cole	Target Version	CMake 2.8.3 =>
2010-09-15 04:28	Marcel Loose	Note Added: 0022223
2010-09-15 04:29	Marcel Loose	Status	assigned => resolved
2010-09-15 17:59	David Cole	Note Added: 0022239
2010-09-15 17:59	David Cole	Status	resolved => assigned
2010-09-20 04:44	Marcel Loose	Note Added: 0022272
2010-09-28 08:39	Marcel Loose	Status	assigned => resolved
2010-11-10 07:40	David Cole	Note Added: 0023128
2010-11-10 07:41	David Cole	Target Version	=> CMake 2.8.4
2010-12-17 11:24	David Cole	Note Added: 0024234
2010-12-17 11:24	David Cole	Status	resolved => feedback
2010-12-20 05:15	Marcel Loose	Note Added: 0024270
2010-12-20 05:15	Marcel Loose	Status	feedback => assigned
2010-12-20 05:17	Marcel Loose	Note Added: 0024271
2010-12-20 05:19	Marcel Loose	Note Added: 0024272
2010-12-20 05:22	Marcel Loose	Note Added: 0024273
2010-12-20 05:23	Marcel Loose	Note Added: 0024274
2010-12-20 05:36	Rolf Eike Beer	Note Added: 0024275
2011-01-04 12:01	David Cole	Note Added: 0024397
2011-01-04 12:01	David Cole	Target Version	CMake 2.8.4 =>
2011-06-24 14:53	Aaron C. Meadows	Tag Attached: FindSubversion
2012-08-28 06:06	Marcel Loose	Status	assigned => backlog
2016-06-10 14:27	Kitware Robot	Note Added: 0041648
2016-06-10 14:27	Kitware Robot	Status	backlog => resolved
2016-06-10 14:27	Kitware Robot	Resolution	reopened => moved
2016-06-10 14:31	Kitware Robot	Status	resolved => closed

Notes
(0021943) David Cole (manager) 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 (manager) 2010-08-25 07:39	Do not want GPL-ed code in CMake.

(0021954) Marcel Loose (developer) 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 (manager) 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 (developer) 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 (manager) 2010-09-07 13:43	To become the FindSubversion.cmake module maintainer, simply follow the directions here: http://www.cmake.org/Wiki/CMake:Module_Maintainers [^] To request git privileges, send us your public key as outlined here: http://www.cmake.org/Wiki/CMake/Git#Publishing [^] http://www.cmake.org/Wiki/CMake/Git#Topic_Stage [^] Send mail to the list saying that you are volunteering to become the maintainer, send us the public side of your ssh key so that we can give you git push privileges, and then you can fix this bug by adding to FindSubversion.cmake, committing and pushing to 'next'. After you do that, and we've had a green dashboard overnight, Brad and I will merge the changes to master for the following release. Thanks!

(0022113) David Cole (manager) 2010-09-07 15:18	Hi Marcel, In addition to the instructions in the previous note, ... to be a CMake module maintainer, you should also be subscribed to the CMake developers mailing list. Sign up for that here: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers [^] Thanks again.

(0022219) David Cole (manager) 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 (developer) 2010-09-15 04:28	Added Subversion_WC_UPDATE macro.

(0022239) David Cole (manager) 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 (developer) 2010-09-20 04:44	Should be OK now. I checked with gitweb.

(0023128) David Cole (manager) 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 (manager) 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 (developer) 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 (developer) 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 (developer) 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 (developer) 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 (developer) 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 (developer) 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 (manager) 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 (administrator) 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.

Relationships