You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by vijayaguru <vi...@collab.net> on 2010/09/08 16:07:19 UTC

[PATCH] Test case for issue #3471: svn up touches file w/ lock & svn:keywords property

Hi,

The attached patch adds a test case for issue #3471:svn up touches file
w/ lock & svn:keywords property and marking it as 'XFail' until the
issue is fixed.
[[[
  Log:
  Add a test for issue #3471:'svn up touches file w/ lock &
svn:keywords  property'

* subversion/tests/cmdline/update_tests.py
   (update_with_file_lock_and_keywords_property_set): New test case.

   (test_list): Add update_with_file_lock_and_keywords_property_set and
mark it as XFail.

Patch by: Vijayaguru G <vi...@collab.net>
]]]


Thanks & Regards,
Vijayaguru


When should working value of svn:keywords apply?

Posted by Philip Martin <ph...@wandisco.com>.
Philip Martin <ph...@wandisco.com> writes:

> vijayaguru <vi...@collab.net> writes:
>
>> In 1.5.x & 1.6.x, the keyword is expanded for the first update and
>> timestamp is changed for further updates. Here We are just locking the
>> file with keywords property set and we are not committing the file.Then
>> running "svn update" should expand the keyword & change the timestamp?
>>
>> But in 1.4.x , the keyword will not expand and timestamp won't
>> change for first update.
>
> It's a change in behaviour but is it a bug?  Why should the keyword
> remain unexpanded?

>From http://subversion.tigris.org/issues/show_bug.cgi?id=3471

Take a plain versioned file and add a keyword:

 $ echo '$Id' >> wc/f
 $ svn ps svn:keywords id wc/f
 $ svn up wc

should update expand the keyword?  It doesn't at present, but if I
lock the file then the expansion happens.

Obviously it's a bug that lock makes a difference.  But which is the
correct behaviour?  To ignore the local property setting or to expand
the keyword?

What about svn:eol-style?  If I do both

 $ svn ps svn:keywords id wc/f
 $ svn ps svn:keywords CRLF wc/f

then update changes the line-endings when the file is locked (even if
the '$Id$' token is not present in the file) but does not change the
line-endings if the file is not locked. In this case svn:eol-style
only has an effect if svn:keywords is also set.

-- 
Philip

When should working value of svn:keywords apply?

Posted by Philip Martin <ph...@wandisco.com>.
Philip Martin <ph...@wandisco.com> writes:

> vijayaguru <vi...@collab.net> writes:
>
>> In 1.5.x & 1.6.x, the keyword is expanded for the first update and
>> timestamp is changed for further updates. Here We are just locking the
>> file with keywords property set and we are not committing the file.Then
>> running "svn update" should expand the keyword & change the timestamp?
>>
>> But in 1.4.x , the keyword will not expand and timestamp won't
>> change for first update.
>
> It's a change in behaviour but is it a bug?  Why should the keyword
> remain unexpanded?

Re: [PATCH] Test case for issue #3471: svn up touches file w/ lock & svn:keywords property

Posted by Philip Martin <ph...@wandisco.com>.
vijayaguru <vi...@collab.net> writes:

> In 1.5.x & 1.6.x, the keyword is expanded for the first update and
> timestamp is changed for further updates. Here We are just locking the
> file with keywords property set and we are not committing the file.Then
> running "svn update" should expand the keyword & change the timestamp?
>
> But in 1.4.x , the keyword will not expand and timestamp won't
> change for first update.

It's a change in behaviour but is it a bug?  Why should the keyword
remain unexpanded?

If you think that expanding the keyword is wrong then the test should
be checking the file content, not the timestamp!

-- 
Philip

Re: [PATCH] Test case for issue #3471: svn up touches file w/ lock & svn:keywords property

Posted by vijayaguru <vi...@collab.net>.
On Mon, 2010-09-13 at 10:53 +0100, Philip Martin wrote:
> Senthil Kumaran S <se...@collab.net> writes:
> 
> > In the previous versions of subversion ie., 1.5.x or 1.4.x the keyword
> > will not expand, but this is not the case with 1.6.x in which it gets
> > expanded for the first update. This is what the issue #3471 is about
> > and the committed patch tests that.
> 
> I'm still confused.  The test creates an unexpanded keyword and then
> runs update and the keyword gets expanded.  Are you saying that update
> should not expand the keyword, because that's the way 1.5 behaved?
> 
> I think it's right that update expands the keyword, and so when the
> timestamp changes that is also correct.
> 
> What is a bug is that a *second* update causes the file's timestamp to
> get changed a second time.  A second update doesn't change the file
> contents and so the timestamp should not change.  But the test doesn't
> run a second update.  If the test did run a second update it could use
> do_sleep_for_timestamps rather than sleep.
> 

In 1.5.x & 1.6.x, the keyword is expanded for the first update and
timestamp is changed for further updates. Here We are just locking the
file with keywords property set and we are not committing the file.Then
running "svn update" should expand the keyword & change the timestamp?

But in 1.4.x , the keyword will not expand and timestamp won't change
for first update. 

Thanks & Regards,
Vijayaguru



Re: [PATCH] Test case for issue #3471: svn up touches file w/ lock & svn:keywords property

Posted by Philip Martin <ph...@wandisco.com>.
Senthil Kumaran S <se...@collab.net> writes:

> In the previous versions of subversion ie., 1.5.x or 1.4.x the keyword
> will not expand, but this is not the case with 1.6.x in which it gets
> expanded for the first update. This is what the issue #3471 is about
> and the committed patch tests that.

I'm still confused.  The test creates an unexpanded keyword and then
runs update and the keyword gets expanded.  Are you saying that update
should not expand the keyword, because that's the way 1.5 behaved?

I think it's right that update expands the keyword, and so when the
timestamp changes that is also correct.

What is a bug is that a *second* update causes the file's timestamp to
get changed a second time.  A second update doesn't change the file
contents and so the timestamp should not change.  But the test doesn't
run a second update.  If the test did run a second update it could use
do_sleep_for_timestamps rather than sleep.

-- 
Philip

Re: [PATCH] Test case for issue #3471: svn up touches file w/ lock & svn:keywords property

Posted by Senthil Kumaran S <se...@collab.net>.
Philip Martin wrote:
>> +  mu_ts_after_update = os.path.getmtime(mu_path)
>> +  if (mu_ts_before_update != mu_ts_after_update):
>> +    print("The timestamp of 'mu' before and after update does not match.")
>> +    raise svntest.Failure
> 
> Why is this an failure?

In the previous versions of subversion ie., 1.5.x or 1.4.x the keyword will not 
expand, but this is not the case with 1.6.x in which it gets expanded for the 
first update. This is what the issue #3471 is about and the committed patch 
tests that.

Yes, the sleep delay is required.

Thank You.
-- 
Senthil Kumaran S
http://www.stylesen.org/

Re: [PATCH] Test case for issue #3471: svn up touches file w/ lock & svn:keywords property

Posted by Philip Martin <ph...@wandisco.com>.
vijayaguru <vi...@collab.net> writes:

> +#----------------------------------------------------------------------
> +# Test for issue #3471 'svn up touches file w/ lock & svn:keywords property'
> +#
> +# Marked as XFail until that issue is fixed.
> +def update_with_file_lock_and_keywords_property_set(sbox):
> +  """update with file lock & keywords property set"""
> +  sbox.build()
> +  
> +  wc_dir = sbox.wc_dir
>  
> +  mu_path = os.path.join(wc_dir, 'A', 'mu')
> +  svntest.main.file_append(mu_path, '$Id$')
> +  svntest.main.run_svn(None, 'ps', 'svn:keywords', 'Id', mu_path)
> +  svntest.main.run_svn(None, 'lock', mu_path)
> +  mu_ts_before_update = os.path.getmtime(mu_path)

The keyword is present but not expanded.

> +  
> +  # Issue #3471 manifests itself here; The timestamp of 'mu' gets updated 
> +  # to the time of the last "svn up".
> +  sbox.simple_update()

That update expands the keyword, so I would expect the timestamp to
change.

> +  mu_ts_after_update = os.path.getmtime(mu_path)
> +  if (mu_ts_before_update != mu_ts_after_update):
> +    print("The timestamp of 'mu' before and after update does not match.")
> +    raise svntest.Failure

Why is this an failure?

Perhaps there should be a second call to update, with a check that the
second doesn't change the timestamp.  With two update
sleep_for_timestamps could be used to introduce the necessary delay to
trigger the bug.

-- 
Philip

Re: [PATCH] Test case for issue #3471: svn up touches file w/ lock & svn:keywords property

Posted by Senthil Kumaran S <se...@collab.net>.
vijayaguru wrote:
> Thanks senthil. The attached patch tests the timestamp changes before
> and after "svn up" and raises failure if they are not same.
> 
> [[[
>   Log:
>   Add a test for issue #3471:'svn up touches file w/ lock &
>   svn:keywords  property'
> 
> * subversion/tests/cmdline/update_tests.py
>    (update_with_file_lock_and_keywords_property_set): New test case.
> 
>    (test_list): Add update_with_file_lock_and_keywords_property_set and
>    mark it as XFail.
> 
> ]]]

Committed in r995388.

Thank You.
-- 
Senthil Kumaran S
http://www.stylesen.org/

Re: [PATCH] Test case for issue #3471: svn up touches file w/ lock & svn:keywords property

Posted by vijayaguru <vi...@collab.net>.
On Thu, 2010-09-09 at 13:08 +0530, Senthil Kumaran S wrote:
> Hi Vijay,
> 
> vijayaguru wrote:
> > The attached patch adds a test case for issue #3471:svn up touches file
> > w/ lock & svn:keywords property and marking it as 'XFail' until the
> > issue is fixed.
> 
> Thanks for the patch. I reviewed it and I could see the patch does not test the 
> actual problem. The issue says the timestamp changes for the file hence 'make' 
> rebuilds stuff. In this case you have tested whether the keyword is expanded or 
> not, which is a valid check for this use case, but apart from that you must 
> also test whether an update happens for the file or not, because if someone 
> fixes the keyword expansion thingy, and still something else causes an update 
> of the file's timestamp, that is also not acceptable, since 1.4.x didn't behave 
> in such a way. Hence do a check for timestamp on the file after an update.
> 
> PS: See that all lines in your patch is within 79 columns.
> 
> Thank You.


Thanks senthil. The attached patch tests the timestamp changes before
and after "svn up" and raises failure if they are not same.

[[[
  Log:
  Add a test for issue #3471:'svn up touches file w/ lock &
  svn:keywords  property'

* subversion/tests/cmdline/update_tests.py
   (update_with_file_lock_and_keywords_property_set): New test case.

   (test_list): Add update_with_file_lock_and_keywords_property_set and
   mark it as XFail.

]]]


Thanks & Regards,
Vijayaguru

Re: [PATCH] Test case for issue #3471: svn up touches file w/ lock & svn:keywords property

Posted by vijayaguru <vi...@collab.net>.
On Thu, 2010-09-09 at 13:08 +0530, Senthil Kumaran S wrote:
> Hi Vijay,
> 
> vijayaguru wrote:
> > The attached patch adds a test case for issue #3471:svn up touches file
> > w/ lock & svn:keywords property and marking it as 'XFail' until the
> > issue is fixed.
> 
> Thanks for the patch. I reviewed it and I could see the patch does not test the 
> actual problem. The issue says the timestamp changes for the file hence 'make' 
> rebuilds stuff. In this case you have tested whether the keyword is expanded or 
> not, which is a valid check for this use case, but apart from that you must 
> also test whether an update happens for the file or not, because if someone 
> fixes the keyword expansion thingy, and still something else causes an update 
> of the file's timestamp, that is also not acceptable, since 1.4.x didn't behave 
> in such a way. Hence do a check for timestamp on the file after an update.
> 
> PS: See that all lines in your patch is within 79 columns.
> 
> Thank You.


Thanks senthil. The attached patch tests the timestamp changes before
and after "svn up" and raises failure if they are not same.

[[[
  Log:
  Add a test for issue #3471:'svn up touches file w/ lock &
  svn:keywords  property'

* subversion/tests/cmdline/update_tests.py
   (update_with_file_lock_and_keywords_property_set): New test case.

   (test_list): Add update_with_file_lock_and_keywords_property_set and
   mark it as XFail.

]]]


Thanks & Regards,
Vijayaguru

Re: [PATCH] Test case for issue #3471: svn up touches file w/ lock & svn:keywords property

Posted by Senthil Kumaran S <se...@collab.net>.
Hi Vijay,

vijayaguru wrote:
> The attached patch adds a test case for issue #3471:svn up touches file
> w/ lock & svn:keywords property and marking it as 'XFail' until the
> issue is fixed.

Thanks for the patch. I reviewed it and I could see the patch does not test the 
actual problem. The issue says the timestamp changes for the file hence 'make' 
rebuilds stuff. In this case you have tested whether the keyword is expanded or 
not, which is a valid check for this use case, but apart from that you must 
also test whether an update happens for the file or not, because if someone 
fixes the keyword expansion thingy, and still something else causes an update 
of the file's timestamp, that is also not acceptable, since 1.4.x didn't behave 
in such a way. Hence do a check for timestamp on the file after an update.

PS: See that all lines in your patch is within 79 columns.

Thank You.
-- 
Senthil Kumaran S
http://www.stylesen.org/