[Monotone-devel] Updated Issue 97 - test diff_on_missing_trailing_newline_at_end_of_file fails on Cygwin (monotone)

[code]

Hello,

The following issue has been updated:

97 - test diff_on_missing_trailing_newline_at_end_of_file fails on Cygwin
Project: monotone
Status: New
Reported by: Stephen Leake
URL: http://code.monotone.ca/p/monotone/issues/97/
Labels:
 Type:Defect
 Priority:Critical

Comments (last first):

# By Stephen Leake, Oct 28, 2010:

> How can "attrs can be set after a commit"?

'mtn attr set'

> Or, more precisely, how's that different from "file contents can be
> changed after a commit"?

I thought it was; apparently I'm wrong.

> Attributes are part of the manifest and thus of course part of the
> revision hash. Do you propose to change this?

No, I was just wrong; attrs are part of the revision hash.

This is discussed in monotone.info; I've just missed it before now.

So the issue resolves to ensuring that attrs are consistently applied
within a project, and specifically for this and other tests.

There is no way to make an automatic execute attr detector get the
'right' answer on all platforms (because there is no 'right' answer), so
that's not a solution for this test. Thus the issue of the execute attr
being set incorrectly on Cygwin (and other platforms) is a separate
issue; I'll file a separate bug report for that.

The attr_init_functions I provide below is a reasonable solution for
this bug. It should be in test_hooks.lua, so all the tests
benefit from it.

We should do the same for attr_init_functions["mtn:manual_merge"], so
future changes to that don't break tests.

Those are the only attr_init_functions in std_hooks.lua

# By Richard Levitte, Oct 27, 2010:

Further analysis shows that, in fact, Cygwin is regarded as unix by 
configure.ac.  This is probably normally a good thing, since it tries to 
emulate unix...  but when it comes to is_executable, maybe that's not the best 
idea.

So we have a choice.  Either add *-pc-cygwin in the case that catches Win32 
platforms in configure.ac, or make is_executable in unix/process.cc a special 
case for Cygwin.

# By Richard Levitte, Oct 27, 2010:

Looking at the source, I'd say that the problem lies in the added lua routine 
called is_executable().  The actual function that implements that routine can 
be found in unix/process.cc and win32/process.cc.  win32/process.cc does 
nothing (it just returns false), while unix/process.cc does what's sensible for 
Unix.

So, the question in the end is, which of those two does Cygwin use?  I'm 
guessing it uses unix/process.cc.  Doesn't seem to be the right thing to do, 
but I don't know enough about Cygwin to have an opinion, all I wanted was to 
point at where the problem should be.

# By Thomas Moschny, Oct 27, 2010:

How can "attrs can be set after a commit"?

Or, more precisely, how's that different from "file contents can be changed 
after a commit"?

Attributes are part of the manifest and thus of course part of the revision 
hash. Do you propose to change this?

# By Stephen Leake, Oct 27, 2010:

this test is also broken in the same way in version 0.48.

overriding the execute attr init function lets the test pass in 
8f507884dab9bab6b439cec51f6adc6d006d3e66:

attr_init_functions["mtn:execute"] =
   function(filename)
        return nil
   end

# By Stephen Leake, Oct 27, 2010:

Steps to reproduce the problem:
-------------------------------

1. checkout nvm.monotone rev 8f507884dab9bab6b439cec51f6adc6d006d3e66
2. configure for Cygwin
3. run test 

Expected result:
----------------
pass

Actual results:
---------------
fail on line 39

The immediate cause of failure is that the revid for the commit is different 
from the expected:

stdout:
#
# old_revision [b0598d59ea7c3b8b4a6f27e5ac80c1337c33f1c3]
#

file13.diff:
#
# old_revision [40ea110392539e19c2d251569a3610c0051e0c64]
#

This may be caused by Cygwin setting the executable attribute, while MinGW and 
Debian do not:

cygwin:
$../../../mtn.exe au get_manifest_of 
format_version "1"

dir ""

   file "file1"
content [a43dc27b1c92cccc533ceb3a27035128e26e5b07]
   attr "mtn:execute" "true"

   file "file2"
content [4f8091d2fa31b0598dd0ff6dc936f8d0b47b59d1]
   attr "mtn:execute" "true"


mingw:
../../../mtn.exe au get_manifest_of 
format_version "1"

dir ""

   file "file1"
content [a43dc27b1c92cccc533ceb3a27035128e26e5b07]

   file "file2"
content [4f8091d2fa31b0598dd0ff6dc936f8d0b47b59d1]

But that should not affect the revid; attrs can be set after a commit, so they 
should not change the revid.

Output of `mtn version --full`:
-------------------------------



--
Issue: http://code.monotone.ca/p/monotone/issues/97/