Add a new presubmit check to enforce automatic svn properties in svn config.

Add a new presubmit check to enforce automatic svn properties in svn config.

This should help simplifying project maintenance.

BUG=none
TEST=none


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=72305

[M-A Ruel, 2011-01-23 03:52:07]

I think having a tag line added when hooks are skipped is a good idea too, will
do in a separate change since it's in depot tools.

[Nico, 2011-01-23 04:00:38]

LG

Is it possible to have per-project svn configs? For me personally,
it's pretty annoying that I have to manually remove the stuff added by
autoprops when committing stuff to unrelated svn checkouts I have on
my machine (just curious…I couldn't find anything, so I'm currently
just doing it manually since I don't commit to these other repos all
that often).

On Sat, Jan 22, 2011 at 7:52 PM,  <maruel@chromium.org> wrote:
> Reviewers: Nico,
>
> Message:
> I think having a tag line added when hooks are skipped is a good idea too,
> will
> do in a separate change since it's in depot tools.
>
> Description:
> Add a new presubmit check to enforce automatic svn properties in svn config.
>
> This should help simplifying project maintenance.
>
> BUG=
> TEST=
>
> Please review this at http://codereview.chromium.org/6241012/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files:
>  M PRESUBMIT.py
>
>
> Index: PRESUBMIT.py
> diff --git a/PRESUBMIT.py b/PRESUBMIT.py
> index
>
65df2245acad080469ceb9c7202cc7a4c2bac924..959654c28ca0a19569b93268bedc5a7b5d427c82
> 100644
> --- a/PRESUBMIT.py
> +++ b/PRESUBMIT.py
> @@ -68,6 +68,49 @@ def _CheckSingletonInHeaders(input_api, output_api,
> source_file_filter):
>         files) ]
>   return []
>
> +
> +def _CheckSubversionConfig(input_api, output_api):
> +  """Verifies the subversion config file is correctly setup.
> +
> +  Checks that autoprops are enabled, returns an error otherwise.
> +  """
> +  join = input_api.os_path.join
> +  if input_api.platform == 'win32':
> +    appdata = input_api.environ.get('APPDATA', '')
> +    if not appdata:
> +      return [output_api.PresubmitError('%APPDATA% is not configured.')]
> +    path = join(appdata, 'Subversion', 'config')
> +  else:
> +    home = input_api.environ.get('HOME', '')
> +    if not home:
> +      return [output_api.PresubmitError('$HOME is not configured.')]
> +    path = join(home, '.subversion', 'config')
> +
> +  error_msg = (
> +      'Please look at http://dev.chromium.org/developers/coding-style to\n'
> +      'configure your subversion configuration file. This enables
> automatic\n'
> +      'properties to simplify the project maintenance.')
> +
> +  try:
> +    lines = open(path, 'r').read().splitlines()
> +    # Make sure auto-props is enabled and check for 2 Chromium standard
> +    # auto-prop.
> +    if (not '*.cc = svn:eol-style=LF' in lines or
> +        not '*.pdf = svn:mime-type=application/pdf' in lines or
> +        not 'enable-auto-props = yes' in lines):
> +      return [
> +          output_api.PresubmitError(
> +              'It looks like you have not configured your subversion config
> '
> +              'file.\n' + error_msg)
> +      ]
> +  except (OSError, IOError):
> +    return [
> +        output_api.PresubmitError(
> +            'Can\'t find your subversion config file.\n' + error_msg)
> +    ]
> +  return []
> +
> +
>  def _CheckConstNSObject(input_api, output_api, source_file_filter):
>   """Checks to make sure no objective-c files have |const NSSomeClass*|."""
>   pattern = input_api.re.compile(r'const\s+NS\w*\s*\*')
> @@ -119,6 +162,7 @@ def _CommonChecks(input_api, output_api):
>       input_api, output_api, source_file_filter=sources))
>   results.extend(_CheckNoInterfacesInBase(
>       input_api, output_api, source_file_filter=sources))
> +  results.extend(_CheckSubversionConfig(input_api, output_api))
>   return results
>
>
>
>
>

[M-A Ruel, 2011-01-23 04:03:09]

On 2011/01/23 04:00:38, Nico wrote:
> LG
> 
> Is it possible to have per-project svn configs? For me personally,
> it's pretty annoying that I have to manually remove the stuff added by
> autoprops when committing stuff to unrelated svn checkouts I have on
> my machine (just curious…I couldn't find anything, so I'm currently
> just doing it manually since I don't commit to these other repos all
> that often).

That's a valid question. I thought about it but there is no real solution I know
that could fix this beside using per repo svn config but that would be messy to
implement consistently across all tools. I'll wait on Monday before committing
anyway in case anyone else wants to comment.

[M-A Ruel, 2011-01-23 21:19:57]

On 2011/01/23 04:03:09, Marc-Antoine Ruel wrote:
> On 2011/01/23 04:00:38, Nico wrote:
> > LG
> > 
> > Is it possible to have per-project svn configs? For me personally,
> > it's pretty annoying that I have to manually remove the stuff added by
> > autoprops when committing stuff to unrelated svn checkouts I have on
> > my machine (just curious…I couldn't find anything, so I'm currently
> > just doing it manually since I don't commit to these other repos all
> > that often).
> 
> That's a valid question. I thought about it but there is no real solution I
know
> that could fix this beside using per repo svn config but that would be messy
to
> implement consistently across all tools. I'll wait on Monday before committing
> anyway in case anyone else wants to comment.

Actually I'll commit right away and will revert if there's any complain in the
next days as I need to commit something to trigger the master.

The check is only triggered on commit and not on upload.

[wtc, 2011-01-31 22:37:36]

http://codereview.chromium.org/6241012/diff/8001/PRESUBMIT.py
File PRESUBMIT.py (right):

http://codereview.chromium.org/6241012/diff/8001/PRESUBMIT.py#newcode104
PRESUBMIT.py:104: 'file.\n' + error_msg)
I got this error message when I tried to commit a CL today.
I was confused because I configured my ~/.subversion/config file
before.

I believe it's because my ~/.subversion/config file didn't
have the '*.pdf = svn:mime-type=application/pdf' line.
I suspect the *.pdf line is a new requirement.  So it may
be better to add ", or your subversion config file is not up-to-date"
to the error message.