You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Masaru Tsuchiyama <m....@gmail.com> on 2013/07/22 16:51:56 UTC
[PATCH] add 'svn:use-commit-times' property
Hi.
I attach a patch to add 'svn:use-commit-times' property and
a perl script to test it.
.
If a file has the property, the timestamp of the file is modified to commit time
when checkout, export, update.
The script is tested on Windows, not on UNIX/Linux.
(I think it works on UNIX/Linux too)
[[[
add 'svn:use-commit-times' property. If a file has 'svn:use-commit-times' property,
the timestamp of the file is modified to commit time.
* subversion/include/svn_props.h
(): add the definition of SVN_PROP_USE_COMMIT_TIMES.
(SVN_PROP_NODE_ALL_PROPS) add SVN_PROP_USE_COMMIT_TIMES to SVN_PROP_NODE_ALL_PROPS.
* subversion/libsvn_wc/workqueue.c
(run_file_install): set use_commit_times flag to TRUE if 'svn:use-commit-times' property is set.
]]]
--
Masaru Tsuchiyama <m....@gmail.com>
Re: [PATCH] add 'svn:use-commit-times' property
Posted by Daniel Shahaf <da...@elego.de>.
Could you generate patches with
svn diff -x-p
, please? That makes review easier.
Thanks.
Re: [PATCH] add 'svn:use-commit-times' property
Posted by Masaru Tsuchiyama <m....@gmail.com>.
Hi.
Does anybody comment my patch?
--
Masaru Tsuchiyama <m....@gmail.com>
Re: [PATCH] add 'svn:use-commit-times' property
Posted by Daniel Shahaf <da...@elego.de>.
Masaru Tsuchiyama wrote on Thu, Aug 22, 2013 at 23:20:14 +0900:
> Could you comment the updated patch?
As I said, I review the style but not the actual feature. Other devs
are far more familiar than I am with the client layer so I prefer to let
someone else do the review. Indeed Philip has now commented on the
actual functionality being proposed.
Re: [PATCH] add 'svn:use-commit-times' property
Posted by Masaru Tsuchiyama <m....@gmail.com>.
Hello Daniel
Could you comment the updated patch?
> I prefer that someone else review the semantics, i.e., the actual
> feature being added.
Does anybody comment for the feature?
--
Masaru Tsuchiyama <m....@gmail.com>
masaru tsuchiyama wrote:
> > You still have several places where your code exceeds 80 characters.
>
> Fixed at attached patch.
>
> >> Encoding problem? I see a Yen symbol where a backslash should be.
>
> What do you see in the attached file? I use Google web mailer.
>
>
>
> 2013/8/8 Daniel Shahaf <danielsh@elego.de <ma...@elego.de>>
>
> Masaru Tsuchiyama wrote on Thu, Aug 08, 2013 at 23:13:54 +0900:
> >>> " before it is modified. Makes the working copy
> file read-only¥n"
> >>> " when it is not locked. Use 'svn propdel
> svn:needs-lock PATH...'¥n"
> >>> " to clear.¥n"
> >>> + " svn:use-commit-times - If present, make the
> timestamp of the file¥n"
> >>> + " to commit time. Use 'svn propdel
> svn:use-commit-times PATH...' to clear.¥n"
> >>> "¥n"
> >>> " Subversion recognizes the following special versioned
> properties on a¥n"
> >>> " directory:¥n"
> >>
> >> Encoding problem? I see a Yen symbol where a backslash should be.
> >
> > On Japanse version of Windows (SJIS), Yen and backslash are same
> symbol.
> > What do you see in the attached file, not inline text?
>
> I tried two versions of mutt and each of them showed something gelse.
>
> I tried to decode the attachment manually, with:
>
> openssl base64 -d | iconv -f shift_jis -t utf-8 | fgrep 'n"'
>
> and that gives me a Yen symbol.
>
> > Fixed.
>
> Thanks. You still have several places where your code exceeds 80
> characters. Otherwise it looks good as far as form is concerned.
>
> I prefer that someone else review the semantics, i.e., the actual
> feature being added.
>
> Cheers,
>
> Daniel
>
>
Re: [PATCH] add 'svn:use-commit-times' property
Posted by Masaru Tsuchiyama <m....@gmail.com>.
Philip Martin wrote:
> masaru tsuchiyama <m....@gmail.com> writes:
>
>> + if (!fb->adding_file)
>> + {
>> + if (fb->use_commit_times && fb->changed_date)
>> + {
>> + SVN_ERR(svn_io_set_file_affected_time(fb->changed_date,
>> + fb->local_abspath,
>> + scratch_pool));
>> + }
>> + else if (fb->remove_use_commit_times)
>> + {
>> + SVN_ERR(svn_io_set_file_affected_time(apr_time_now(),
>> + fb->local_abspath,
>> + scratch_pool));
>> + }
>> + }
>> +
> The call to apr_time_now still looks wrong to me. If I understand the
> patch it causes the timestamp to be updated to the current time when
> svn:use-commit-times is removed. I don't know how this feature is
> supposed to be used but I would expect the timestamp to be unchanged
> when the property is removed. Is setting it to the current time
> useful behaviour?
I thought if a file with svn:use-commit-times was an input file for
build system,
it was convenient to force process the file by updating the timestamp.
But it may not need to update the timestamp.
Regards.
--
Masaru Tsuchiyama <m....@gmail.com>
Re: [PATCH] add 'svn:use-commit-times' property
Posted by Philip Martin <ph...@wandisco.com>.
masaru tsuchiyama <m....@gmail.com> writes:
> + if (!fb->adding_file)
> + {
> + if (fb->use_commit_times && fb->changed_date)
> + {
> + SVN_ERR(svn_io_set_file_affected_time(fb->changed_date,
> + fb->local_abspath,
> + scratch_pool));
> + }
> + else if (fb->remove_use_commit_times)
> + {
> + SVN_ERR(svn_io_set_file_affected_time(apr_time_now(),
> + fb->local_abspath,
> + scratch_pool));
> + }
> + }
> +
The call to apr_time_now still looks wrong to me. If I understand the
patch it causes the timestamp to be updated to the current time when
svn:use-commit-times is removed. I don't know how this feature is
supposed to be used but I would expect the timestamp to be unchanged
when the property is removed. Is setting it to the current time
useful behaviour?
--
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*
Re: [PATCH] add 'svn:use-commit-times' property
Posted by masaru tsuchiyama <m....@gmail.com>.
> You still have several places where your code exceeds 80 characters.
Fixed at attached patch.
>> Encoding problem? I see a Yen symbol where a backslash should be.
What do you see in the attached file? I use Google web mailer.
2013/8/8 Daniel Shahaf <da...@elego.de>
> Masaru Tsuchiyama wrote on Thu, Aug 08, 2013 at 23:13:54 +0900:
> >>> " before it is modified. Makes the working copy file
> read-only¥n"
> >>> " when it is not locked. Use 'svn propdel svn:needs-lock
> PATH...'¥n"
> >>> " to clear.¥n"
> >>> + " svn:use-commit-times - If present, make the timestamp of
> the file¥n"
> >>> + " to commit time. Use 'svn propdel svn:use-commit-times
> PATH...' to clear.¥n"
> >>> "¥n"
> >>> " Subversion recognizes the following special versioned
> properties on a¥n"
> >>> " directory:¥n"
> >>
> >> Encoding problem? I see a Yen symbol where a backslash should be.
> >
> > On Japanse version of Windows (SJIS), Yen and backslash are same symbol.
> > What do you see in the attached file, not inline text?
>
> I tried two versions of mutt and each of them showed something gelse.
>
> I tried to decode the attachment manually, with:
>
> openssl base64 -d | iconv -f shift_jis -t utf-8 | fgrep 'n"'
>
> and that gives me a Yen symbol.
>
> > Fixed.
>
> Thanks. You still have several places where your code exceeds 80
> characters. Otherwise it looks good as far as form is concerned.
>
> I prefer that someone else review the semantics, i.e., the actual
> feature being added.
>
> Cheers,
>
> Daniel
>
Re: [PATCH] add 'svn:use-commit-times' property
Posted by Daniel Shahaf <da...@elego.de>.
Masaru Tsuchiyama wrote on Thu, Aug 08, 2013 at 23:13:54 +0900:
>>> " before it is modified. Makes the working copy file read-only¥n"
>>> " when it is not locked. Use 'svn propdel svn:needs-lock PATH...'¥n"
>>> " to clear.¥n"
>>> + " svn:use-commit-times - If present, make the timestamp of the file¥n"
>>> + " to commit time. Use 'svn propdel svn:use-commit-times PATH...' to clear.¥n"
>>> "¥n"
>>> " Subversion recognizes the following special versioned properties on a¥n"
>>> " directory:¥n"
>>
>> Encoding problem? I see a Yen symbol where a backslash should be.
>
> On Japanse version of Windows (SJIS), Yen and backslash are same symbol.
> What do you see in the attached file, not inline text?
I tried two versions of mutt and each of them showed something gelse.
I tried to decode the attachment manually, with:
openssl base64 -d | iconv -f shift_jis -t utf-8 | fgrep 'n"'
and that gives me a Yen symbol.
> Fixed.
Thanks. You still have several places where your code exceeds 80
characters. Otherwise it looks good as far as form is concerned.
I prefer that someone else review the semantics, i.e., the actual
feature being added.
Cheers,
Daniel
Re: [PATCH] add 'svn:use-commit-times' property
Posted by Masaru Tsuchiyama <m....@gmail.com>.
>> +/** The value to force the executable property to when set.
>> + *
>> + * @deprecated Provided for backward compatibility with the 1.4 API.
>> + * Use @c SVN_PROP_BOOLEAN_TRUE instead.
removed this.
> Fix line breaks, wrap to 80 columns.
fixed this.
> Please make the comments say more than the struct member name does.
added comments.
> Wrong indentation. The 'if' (including its block) should be two spaces
> fewer indented than it is.
fixed indentation.
> "mutually exclusive"
fixed.
>> + SVN_ERR_ASSERT( (fb->use_commit_times & fb->remove_use_commit_times) == 0 );
>
> Use boolean-and, not bitwise.
Fixed the logic.
>> + if ( !fb->adding_file )
>
> No whitespace between parentheses and condition.
Fixed.
>> " before it is modified. Makes the working copy file read-only･n"
>> " when it is not locked. Use 'svn propdel svn:needs-lock PATH...'･n"
>> " to clear.･n"
>> + " svn:use-commit-times - If present, make the timestamp of the file･n"
>> + " to commit time. Use 'svn propdel svn:use-commit-times PATH...' to clear.･n"
>> "･n"
>> " Subversion recognizes the following special versioned properties on a･n"
>> " directory:･n"
>
> Encoding problem? I see a Yen symbol where a backslash should be.
On Japanse version of Windows (SJIS), Yen and backslash are same symbol.
What do you see in the attached file, not inline text?
>> + 'svn:date', '2001-01-01T00:00:00.000000Z',
>> + secs_svn_date = time.mktime( (2001, 1, 1, 0, 0, 0, 0, 0, 0) )
>
> Personally I wouldn't use all-zeroes as the test value, that's too
> likely to false negative the test (ie, pass when there's a problem).
changed the date.
> Please add a trailing comma.
Fixed.
--
Masaru Tsuchiyama <m....@gmail.com>
Re: [PATCH] add 'svn:use-commit-times' property
Posted by Daniel Shahaf <da...@elego.de>.
A review mostly of the code formatting (whitespace etc). In particular
I'm not reviewing the feature being added:
> Index: subversion/include/svn_props.h
> ===================================================================
> --- subversion/include/svn_props.h (revision 1509957)
> +++ subversion/include/svn_props.h (working copy)
> @@ -412,6 +412,14 @@ svn_prop_name_is_valid(const char *prop_name);
> */
> #define SVN_PROP_SPECIAL_VALUE SVN_PROP_BOOLEAN_TRUE
>
> +/** The value to force the executable property to when set.
> + *
> + * @deprecated Provided for backward compatibility with the 1.4 API.
> + * Use @c SVN_PROP_BOOLEAN_TRUE instead.
That's still wrong. It does not make sense to describe this symbol as
"for compatibility with 1.4" since /tags/1.4.0/subversion/include/
doesn't have it.
Do you understand what @deprecated means? It's used to mark API
entities that were added in a prior minor (1.x.0) release that are
scheduled to be removed in the next major (x.0.0) release, so it doesn't
make sense to create an API deprecated (only to deprecate an API that
has appeared in a minor release).
> + * @since New in 1.9.
> + */
> +#define SVN_PROP_USE_COMMIT_TIMES_VALUE SVN_PROP_BOOLEAN_TRUE
> @@ -449,6 +457,12 @@ svn_prop_name_is_valid(const char *prop_name);
> +/**
> + * Property used to control whether timestamp of a file is modified to the commit time * @since New in 1.9.
Fix line breaks, wrap to 80 columns.
> + * @since New in 1.9.
> + */
> +#define SVN_PROP_USE_COMMIT_TIMES SVN_PROP_PREFIX "use-commit-times"
> +++ subversion/libsvn_wc/update_editor.c (working copy)
> @@ -751,6 +751,12 @@ struct file_baton
>
> /* The tree conflict to install once the node is really edited */
> svn_skel_t *edit_conflict;
> +
> + /* use-commit-times */
> + svn_boolean_t use_commit_times;
> +
> + /* remove use-commit-times */
> + svn_boolean_t remove_use_commit_times;
Please make the comments say more than the struct member name does.
> @@ -3830,6 +3836,17 @@ change_file_prop(void *file_baton,
> fb->already_notified = TRUE;
> }
> }
> + if (strcmp(name, SVN_PROP_USE_COMMIT_TIMES) == 0)
> + {
> + if (propchange->value)
Wrong indentation. The 'if' (including its block) should be two spaces
fewer indented than it is.
> + {
> + fb->use_commit_times = TRUE;
> + }
> + else
> + {
> + fb->remove_use_commit_times = TRUE;
> + }
> + }
>
> return SVN_NO_ERROR;
> }
> @@ -4636,6 +4653,24 @@ close_file(void *file_baton,
> eb->notify_func(eb->notify_baton, notify, scratch_pool);
> }
>
> + /* use_commit_times and remove_use_commit_times are mutably exclusive */
"mutually exclusive"
> + SVN_ERR_ASSERT( (fb->use_commit_times & fb->remove_use_commit_times) == 0 );
Use boolean-and, not bitwise.
> + if ( !fb->adding_file )
No whitespace between parentheses and condition.
> " before it is modified. Makes the working copy file read-only¥n"
> " when it is not locked. Use 'svn propdel svn:needs-lock PATH...'¥n"
> " to clear.¥n"
> + " svn:use-commit-times - If present, make the timestamp of the file¥n"
> + " to commit time. Use 'svn propdel svn:use-commit-times PATH...' to clear.¥n"
> "¥n"
> " Subversion recognizes the following special versioned properties on a¥n"
> " directory:¥n"
Encoding problem? I see a Yen symbol where a backslash should be.
> + 'svn:date', '2001-01-01T00:00:00.000000Z',
> + secs_svn_date = time.mktime( (2001, 1, 1, 0, 0, 0, 0, 0, 0) )
Personally I wouldn't use all-zeroes as the test value, that's too
likely to false negative the test (ie, pass when there's a problem).
> - checkout_wc_from_drive
> + checkout_wc_from_drive,
> + checkout_with_use_commit_times
Please add a trailing comma.
> ]
Re: [PATCH] add 'svn:use-commit-times' property
Posted by Masaru Tsuchiyama <m....@gmail.com>.
> Use SVN_ERR_ASSERT instead.
I fixed this in the attached patch.
> The apr_time_now call looks odd. Do we need to change the file's
> timestamp in this case?
Do you think the file's fimestamp should be preserved afer removing the
property?
> You also need to update the help text for 'svn ps' to describe the new
> property, see subversion/svn/svn.c.
I fixed this in the attached patch.
> and "if (" not "if(".
I fixed this in the attached patch.
> svn diff -x-p
> , please? That makes review easier.
I fixed this in the attached patch.
> Inheriting could be a later patch. Another issue in the current patch
> is the use of apr_time_now when the property is removed; changing that
> affects the regression test.
I added a test in update_with_use_commit_times() at
subversion/tests/cmdline/update_tests.py.
Regards.
--
Masaru Tsuchiyama<m....@gmail.com>
Re: [PATCH] add 'svn:use-commit-times' property
Posted by Philip Martin <ph...@wandisco.com>.
Daniel Shahaf <da...@elego.de> writes:
> Philip Martin wrote on Wed, Jul 31, 2013 at 15:09:10 +0100:
>> Perhaps this property should be inheritable? Then it could be set on a
>> directory and apply to the whole tree?
>
> Even if yes, can't inheritability be implemented in a future patch?
> i.e., maybe we should commit this patch and then think about
> inheritability?
Inheriting could be a later patch. Another issue in the current patch
is the use of apr_time_now when the property is removed; changing that
affects the regression test.
--
Philip Martin | Subversion Committer
WANdisco | Non-Stop Data
Re: [PATCH] add 'svn:use-commit-times' property
Posted by Daniel Shahaf <da...@elego.de>.
Philip Martin wrote on Wed, Jul 31, 2013 at 15:09:10 +0100:
> Perhaps this property should be inheritable? Then it could be set on a
> directory and apply to the whole tree?
Even if yes, can't inheritability be implemented in a future patch?
i.e., maybe we should commit this patch and then think about
inheritability?
(I have no opinion on this functionality, i.e., on whether or not we
should commit the standing patch.)
Re: [PATCH] add 'svn:use-commit-times' property
Posted by Ben Reser <be...@reser.org>.
On Wed, Jul 31, 2013 at 11:19 PM, Markus Schaber <m....@codesys.com> wrote:
> Using another property is not compatible with inheritance, as it will be difficult to override the setting again in deeper nested directories.
>
> What about disabling the feature when the property contains the test "false"?
+1
Inheritable properties need to behave more like the boolean support in
our config rather than the existing property boolean support. I.E.
explicit true and false values with a default value if a value isn't
specified.
I think it'd be best to introduce this new style of boolean property
only to new svn: properties that we add. Leave the existing ones as
empty or "*" (actually any value but it's folded to "*" by our
client). This avoids any breakage with existing users, though I
really don't expect that anyone would set the value to "false"
expecting it to be treated as true.
AW: [PATCH] add 'svn:use-commit-times' property
Posted by Markus Schaber <m....@codesys.com>.
Hi, Martin,
> Von: Philip Martin [mailto:philip.martin@wandisco.com]
> Masaru Tsuchiyama <m....@gmail.com> writes:
> [svn:use_commit_times property]
> Perhaps this property should be inheritable? Then it could be set on a
> directory and apply to the whole tree? I imagine users will want that but it
> raises a new problem: how would a node override the inherited property? It
> can't delete a property that isn't set, and it can't set it as that enables
> use-commit-times. How would we make inheriting work?
> Perhaps another property svn:no-commit-times?
Using another property is not compatible with inheritance, as it will be difficult to override the setting again in deeper nested directories.
What about disabling the feature when the property contains the test "false"?
Best regards
Markus Schaber
CODESYS(r) a trademark of 3S-Smart Software Solutions GmbH
Inspiring Automation Solutions
3S-Smart Software Solutions GmbH
Dipl.-Inf. Markus Schaber | Product Development Core Technology
Memminger Str. 151 | 87439 Kempten | Germany
Tel. +49-831-54031-979 | Fax +49-831-54031-50
E-Mail: m.schaber@codesys.com | Web: http://www.codesys.com | CODESYS store: http://store.codesys.com
CODESYS forum: http://forum.codesys.com
Managing Directors: Dipl.Inf. Dieter Hess, Dipl.Inf. Manfred Werner | Trade register: Kempten HRB 6186 | Tax ID No.: DE 167014915
Re: [PATCH] add 'svn:use-commit-times' property
Posted by Philip Martin <ph...@wandisco.com>.
Masaru Tsuchiyama <m....@gmail.com> writes:
> Index: subversion/include/svn_props.h
> ===================================================================
> --- subversion/include/svn_props.h (revision 1507762)
> +++ subversion/include/svn_props.h (working copy)
> @@ -412,6 +412,14 @@
> */
> #define SVN_PROP_SPECIAL_VALUE SVN_PROP_BOOLEAN_TRUE
>
> +/** The value to force the executable property to when set.
> + *
> + * @deprecated Provided for backward compatibility with the 1.4 API.
> + * Use @c SVN_PROP_BOOLEAN_TRUE instead.
> + * @since New in 1.9.
> + */
> +#define SVN_PROP_USE_COMMIT_TIMES_VALUE SVN_PROP_BOOLEAN_TRUE
> +
We don't want that, no existing code can be using it.
> /** Describes external items to check out into this directory.
> *
> * The format is a series of lines, each in the following format:
> @@ -449,6 +457,12 @@
> /** Property used to record inheritable configuration ignores. */
> #define SVN_PROP_INHERITABLE_IGNORES SVN_PROP_PREFIX "global-ignores"
>
> +/**
> + * Property used to control whether timestamp of a file is modified to the commit time * @since New in 1.9.
> + * @since New in 1.9.
> + */
> +#define SVN_PROP_USE_COMMIT_TIMES SVN_PROP_PREFIX "use-commit-times"
> +
> /** Meta-data properties.
> *
> * The following properties are used for storing meta-data about
> @@ -509,7 +523,8 @@
> SVN_PROP_TEXT_TIME, \
> SVN_PROP_OWNER, \
> SVN_PROP_GROUP, \
> - SVN_PROP_UNIX_MODE,
> + SVN_PROP_UNIX_MODE, \
> + SVN_PROP_USE_COMMIT_TIMES,
>
> /** @} */
>
> Index: subversion/libsvn_subr/properties.c
> ===================================================================
> --- subversion/libsvn_subr/properties.c (revision 1507762)
> +++ subversion/libsvn_subr/properties.c (working copy)
> @@ -444,7 +444,8 @@
> make any speed difference. */
> if (strcmp(prop_name, SVN_PROP_EXECUTABLE) == 0
> || strcmp(prop_name, SVN_PROP_NEEDS_LOCK) == 0
> - || strcmp(prop_name, SVN_PROP_SPECIAL) == 0)
> + || strcmp(prop_name, SVN_PROP_SPECIAL) == 0
> + || strcmp(prop_name, SVN_PROP_USE_COMMIT_TIMES) == 0)
> return TRUE;
> return FALSE;
> }
> Index: subversion/libsvn_wc/props.c
> ===================================================================
> --- subversion/libsvn_wc/props.c (revision 1507762)
> +++ subversion/libsvn_wc/props.c (working copy)
> @@ -2153,7 +2153,7 @@
> }
> else if (svn_prop_is_boolean(propname))
> {
> - /* SVN_PROP_EXECUTABLE, SVN_PROP_NEEDS_LOCK, SVN_PROP_SPECIAL */
> + /* SVN_PROP_EXECUTABLE, SVN_PROP_NEEDS_LOCK, SVN_PROP_SPECIAL, SVN_PROP_USE_COMMIT_TIMES */
> propval = &boolean_value;
> }
> else if (strcmp(propname, SVN_PROP_MERGEINFO) == 0)
> Index: subversion/libsvn_wc/update_editor.c
> ===================================================================
> --- subversion/libsvn_wc/update_editor.c (revision 1507762)
> +++ subversion/libsvn_wc/update_editor.c (working copy)
> @@ -23,6 +23,7 @@
>
>
>
> +#include <assert.h>
Avoid assert if possible.
> #include <stdlib.h>
> #include <string.h>
>
> @@ -751,6 +752,12 @@
>
> /* The tree conflict to install once the node is really edited */
> svn_skel_t *edit_conflict;
> +
> + /* use-commit-times */
> + svn_boolean_t use_commit_times;
> +
> + /* remove use-commit-times */
> + svn_boolean_t remove_use_commit_times;
> };
>
>
> @@ -3830,6 +3837,17 @@
> fb->already_notified = TRUE;
> }
> }
> + if ( strcmp(name, SVN_PROP_USE_COMMIT_TIMES) == 0)
The whitespace is wrong, use
if (foo)
not
if ( foo )
> + {
> + if( propchange->value )
and "if (" not "if(".
> + {
> + fb->use_commit_times = TRUE;
> + }
> + else
> + {
> + fb->remove_use_commit_times = TRUE;
> + }
> + }
>
> return SVN_NO_ERROR;
> }
> @@ -4636,6 +4654,24 @@
> eb->notify_func(eb->notify_baton, notify, scratch_pool);
> }
>
> + /* use_commit_times and remove_use_commit_times are mutably exclusive */
> + assert( (fb->use_commit_times & fb->remove_use_commit_times) == 0 );
Use SVN_ERR_ASSERT instead.
> + if ( !fb->adding_file )
> + {
> + if ( fb->use_commit_times && fb->changed_date )
> + {
> + SVN_ERR(svn_io_set_file_affected_time(fb->changed_date,
> + fb->local_abspath,
> + scratch_pool));
> + }
> + else if ( fb->remove_use_commit_times )
> + {
> + SVN_ERR(svn_io_set_file_affected_time(apr_time_now(),
> + fb->local_abspath,
> + scratch_pool));
> + }
The apr_time_now call looks odd. Do we need to change the file's
timestamp in this case?
> + }
> +
> svn_pool_destroy(fb->pool); /* Destroy scratch_pool */
>
> /* We have one less referrer to the directory */
You also need to update the help text for 'svn ps' to describe the new
property, see subversion/svn/svn.c.
Perhaps this property should be inheritable? Then it could be set on a
directory and apply to the whole tree? I imagine users will want that
but it raises a new problem: how would a node override the inherited
property? It can't delete a property that isn't set, and it can't set
it as that enables use-commit-times. How would we make inheriting work?
Perhaps another property svn:no-commit-times?
--
Philip Martin | Subversion Committer
WANdisco | Non-Stop Data
Re: [PATCH] add 'svn:use-commit-times' property
Posted by Masaru Tsuchiyama <m....@gmail.com>.
Hi.
I attache new patch.
[[[
add 'svn:use-commit-times' property. If a file has 'svn:use-commit-times' property,
the timestamp of the file is modified to commit time.
* subversion/include/svn_props.h
(): add SVN_PROP_USE_COMMIT_TIMES and SVN_PROP_USE_COMMIT_TIMES_VALUE
(): add SVN_PROP_USE_COMMIT_TIMES to SVN_PROP_NODE_ALL_PROPS
* subversion/libsvn_subr/properties.c
(svn_prop_is_boolean) : svn_prop_is_boolean() checks SVN_PROP_USE_COMMIT_TIMES
* subversion/libsvn_wc/props.c
(svn_wc_canonicalize_svn_prop) : add comment
* subversion/libsvn_wc/update_editor.c
(): include assert.h
(struct file_baton): add use_commit_times and remove_use_commit_times
(change_file_prop): set fb->use_commit_times or fb->remove_use_commit_times
(close_file): change the timestamp of file when use_commit_times or remove_use_commit_times is set.
* subversion/libsvn_wc/workqueue.c
(run_file_install): set use_commit_times when svn:use-commit-times is set
* subversion/tests/cmdline/checkout_tests.py
(checkout_with_use_commit_times): add checkout test when svn:use-commit-times is set
(test_list): add checkout_with_use_commit_times to test_list
* subversion/tests/cmdline/export_tests.py
(): import time
(export_with_use_commit_times): add export test when svn:use-commit-times is set
(test_list): add export_with_use_commit_times to test_list
* subversion/tests/cmdline/prop_tests.py
(prop_value_conversions): add test setting and getting svn:use-commit-times
* subversion/tests/cmdline/update_tests.py
(update_with_use_commit_times): add update test when svn:use-commit-times is set
(test_list): add update_with_use_commit_times to test_list
]]]
> For consistency with other properties you need to implement this
> behaviour:
>
> /** Properties whose values are interpreted as booleans (such as
> * svn:executable, svn:needs_lock, and svn:special) always fold their
> * value to this.
> *
> * @since New in 1.5.
> */
> #define SVN_PROP_BOOLEAN_TRUE "*"
>
> This probably means changing svn_prop_is_boolean so that
>
> svn propset svn:use-commit-times foo some/file
>
> converts the value 'foo' to SVN_PROP_BOOLEAN_TRUE.
I implement this behavior.
> We also need regression tests for the standard testsuite, see
> subversion/tests/cmdline/prop_tests.py.
I implement the regression tests.
Re: [PATCH] add 'svn:use-commit-times' property
Posted by Philip Martin <ph...@wandisco.com>.
Masaru Tsuchiyama <m....@gmail.com> writes:
> Index: subversion/include/svn_props.h
> ===================================================================
> --- subversion/include/svn_props.h (revision 1505676)
> +++ subversion/include/svn_props.h (working copy)
> @@ -449,6 +449,9 @@
> /** Property used to record inheritable configuration ignores. */
> #define SVN_PROP_INHERITABLE_IGNORES SVN_PROP_PREFIX "global-ignores"
>
> +/** Property used to control whether timestamp of a file is modified to the commit time */
> +#define SVN_PROP_USE_COMMIT_TIMES SVN_PROP_PREFIX "use-commit-times"
> +
You need to mark this new value @since 1.9.
> /** Meta-data properties.
> *
> * The following properties are used for storing meta-data about
> @@ -509,7 +512,8 @@
> SVN_PROP_TEXT_TIME, \
> SVN_PROP_OWNER, \
> SVN_PROP_GROUP, \
> - SVN_PROP_UNIX_MODE,
> + SVN_PROP_UNIX_MODE, \
> + SVN_PROP_USE_COMMIT_TIMES,
>
> /** @} */
>
> Index: subversion/libsvn_wc/workqueue.c
> ===================================================================
> --- subversion/libsvn_wc/workqueue.c (revision 1505676)
> +++ subversion/libsvn_wc/workqueue.c (working copy)
> @@ -638,6 +638,9 @@
> SVN_ERR(svn_io_set_file_read_only(local_abspath, FALSE, scratch_pool));
> }
>
> + if (props && svn_hash_gets(props, SVN_PROP_USE_COMMIT_TIMES))
> + use_commit_times = TRUE;
> +
> if (use_commit_times)
> {
> if (changed_date)
For consistency with other properties you need to implement this
behaviour:
/** Properties whose values are interpreted as booleans (such as
* svn:executable, svn:needs_lock, and svn:special) always fold their
* value to this.
*
* @since New in 1.5.
*/
#define SVN_PROP_BOOLEAN_TRUE "*"
This probably means changing svn_prop_is_boolean so that
svn propset svn:use-commit-times foo some/file
converts the value 'foo' to SVN_PROP_BOOLEAN_TRUE.
We also need regression tests for the standard testsuite, see
subversion/tests/cmdline/prop_tests.py.
--
Philip Martin | Subversion Committer
WANdisco | Non-Stop Data