Merge lp:~gary/z3c.recipe.filetemplate/support-system-python into lp:~gary/z3c.recipe.filetemplate/trunk

Support the new zc.buildout 1.4.0+ include-site-packages option, and more.

--------
Features
--------

- Support the new zc.buildout 1.4.0+ include-site-packages option.

- Use the new zc.buildout 1.4.0+ path sorting algorithm, which puts
  site-package dependency paths after the other dependency paths (but before
  extra paths, as before). This can reduce or eliminate problems with
  packages in site-packages causing other dependencies to be masked with the
  versions in site-packages.

- Support escaping "$" with "$$" in templates. This is particularly useful
  for *NIX shell scripts.

- Support specifying local options in templates without braces (e.g.,
  "Hello $world" is now equivalent to "Hello ${world}".

-----
Fixes
-----

- Clarify that the recipe does not support the relative-paths options.

- Make tests less susceptible to timing errors.

Sorry, this change was removed from the branch, but I mistakenly added it to the comment. (I removed it because, upon trying it in Launchpad, I decided that it caused annoyances for little gain).

NOT PART OF THIS BRANCH:

- Support specifying local options in templates without braces (e.g.,
  "Hello $world" is now equivalent to "Hello ${world}".

On July 11, 2009, Gary Poster wrote:
> Gary Poster has proposed merging
> lp:~gary/z3c.recipe.filetemplate/support-system-python into
> lp:~gary/z3c.recipe.filetemplate/trunk.
>
> Requested reviews:
> Francis J. Lacoste (flacoste)
>
> Support the new zc.buildout 1.4.0+ include-site-packages option, and more.
>

Hi Gary,

This all looks good. My only concern is the new write_and_wait which I don't
understand why it's necessary.

> === modified file 'z3c/recipe/filetemplate/__init__.py'

> @@ -195,17 +202,19 @@
> 'Destinations already exist: %s. Please make sure that '
> 'you really want to generate these automatically. Then '
> 'move them away.', ', '.join(already_exists))
> + seen = [] # we throw this away right now, but could move template

Sentence should start with a capital.

> + # processing up to __init__ if valuable. That would mean that
templates
> + # would be rewritten even if a value in another section had been
> + # referenced; however, it would also mean that __init__ would do
> + # virtually all of the work, with install only doing the writing.
> for rel_path, last_mod, st_mode in self.actions:
> source = os.path.join(self.source_dir, rel_path)
> dest = os.path.join(self.destination_dir, rel_path[:-3])
> mode=stat.S_IMODE(st_mode)
> - template=open(source).read()
> - template=re.sub(r"\$\{([^:]+?)\}", r"${%s:\1}" % self.name,
> - template)
> - self._create_paths(os.path.dirname(dest))
> # we process the file first so that it won't be created if
there
> # is a problem.
> - processed = self.options._sub(template, [])
> + processed = Template(source).substitute(self, seen)
> + self._create_paths(os.path.dirname(dest))
> result=open(dest, "wt")
> result.write(processed)
> result.close()
> @@ -221,3 +230,74 @@
>
> def update(self):
> pass
> +
> +
> +class Template:
> + # hacked from string.Template
> + pattern = re.compile(r"""
> + \$(?:
> + \${(?P<escaped>[^}]*)} | # Escape sequence of two
delimiters.
> + {(?P<braced_single>[-a-z0-9 ._]+)} |
> + # Delimiter and a braced local
option
> + {(?P<braced_double>[-a-z0-9 ._]+:[-a-z0-9 ._]+)} |
> + # Delimiter and a braced fully
> + # qualified option (that is, with
> + # explicit section).
> + {(?P<invalid>[^}]*}) # Other ill-formed delimiter
exprs.
> + )
> + """, re.IGNORECASE | re.VERBOSE)
> +
> + def __init__(self, source):
> + self.source = source
> + self.template = open(source).read()
> +
> + def _get_colno_lineno(self, i):
> + lines = self.template[:i].splitlines(True)
> + if not lines:
> + colno = 1
> + lineno = 1
> + else:
> + colno = i - len(''.join(lines[:-1]))
> + ...

On Jul 13, 2009, at 5:21 PM, Francis J. Lacoste wrote:

> On July 11, 2009, Gary Poster wrote:
>> Gary Poster has proposed merging
>> lp:~gary/z3c.recipe.filetemplate/support-system-python into
>> lp:~gary/z3c.recipe.filetemplate/trunk.
>>
>> Requested reviews:
>> Francis J. Lacoste (flacoste)
>>
>> Support the new zc.buildout 1.4.0+ include-site-packages option,
>> and more.
>>
>
> Hi Gary,
>
> This all looks good. My only concern is the new write_and_wait which
> I don't
> understand why it's necessary.

Cool. I'll discuss below.

>
>> === modified file 'z3c/recipe/filetemplate/__init__.py'
>
>> @@ -195,17 +202,19 @@
>> 'Destinations already exist: %s. Please make sure
>> that '
>> 'you really want to generate these automatically.
>> Then '
>> 'move them away.', ', '.join(already_exists))
>> + seen = [] # we throw this away right now, but could move
>> template
>
> Sentence should start with a capital.

Ack. See incremental diff.

>
>> + # processing up to __init__ if valuable. That would mean
>> that
> templates
>> + # would be rewritten even if a value in another section
>> had been
>> + # referenced; however, it would also mean that __init__
>> would do
>> + # virtually all of the work, with install only doing the
>> writing.
>> for rel_path, last_mod, st_mode in self.actions:
>> source = os.path.join(self.source_dir, rel_path)
>> dest = os.path.join(self.destination_dir, rel_path[:-3])
>> mode=stat.S_IMODE(st_mode)
>> - template=open(source).read()
>> - template=re.sub(r"\$\{([^:]+?)\}", r"${%s:\1}" %
>> self.name,
>> - template)
>> - self._create_paths(os.path.dirname(dest))
>> # we process the file first so that it won't be created
>> if
> there
>> # is a problem.
>> - processed = self.options._sub(template, [])
>> + processed = Template(source).substitute(self, seen)
>> + self._create_paths(os.path.dirname(dest))
>> result=open(dest, "wt")
>> result.write(processed)
>> result.close()
>> @@ -221,3 +230,74 @@
>>
>> def update(self):
>> pass
>> +
>> +
>> +class Template:
>> + # hacked from string.Template
>> + pattern = re.compile(r"""
>> + \$(?:
>> + \${(?P<escaped>[^}]*)} | # Escape sequence
>> of two
> delimiters.
>> + {(?P<braced_single>[-a-z0-9 ._]+)} |
>> + # Delimiter and a braced
>> local
> option
>> + {(?P<braced_double>[-a-z0-9 ._]+:[-a-z0-9 ._]+)} |
>> + # Delimiter and a braced
>> fully
>> + # qualified option (that
>> is, with
>> + # explicit section).
>> + {(?P<invalid>[^}]*}) # Other ill-formed
>> delimiter
> exprs.
>> + )
>> + """, re.IGNORECASE | re.VERBOSE)
>> +
>> + def __init__(self, source):
>> + self.source = source
>> + self.template = open(source)....

On July 13, 2009, Gary Poster wrote:
> In general, the necessity is caused by the mtime precision. On my
> Mac, the precision is 1 second. Here's an example run of a slightly
> modified version of the helper (adding a print), in the interactive
> prompt.
>
> >>> def write_and_wait(dir, *args):
> ... path = os.path.join(dir, *(args[:-1]))
> ... original = os.stat(path).st_mtime
> ... while os.stat(path).st_mtime == original:
> ... print original
> ... f = open(path, 'w')
> ... f.write(args[-1])
> ... f.flush()
> ... os.fsync(f.fileno())
> ... f.close()
> ...
> >>> f = open('/Users/gary/dev/deleteme', 'w')
> >>> f.write('test')
> >>> f.close()
> >>> write_and_wait('/Users/gary/dev/deleteme', 'test')
> 1247536625.0
> >>> write_and_wait('/Users/gary/dev/deleteme', 'test')
> 1247536627.0
> >>> write_and_wait('/Users/gary/dev/deleteme', 'test')
> 1247536628.0
> >>> write_and_wait('/Users/gary/dev/deleteme', 'test')
> 1247536629.0
> 1247536629.0
> 1247536629.0
> [...there are 299 of these...]
> >>>
>
> So, the precision is the point. I tried a variety of things to
> trigger the mtime, but writing an empty string didn't work, and
> directly writing the mtime is OS-specific/dependent, and I didn't
> really want to go there. This is a test harness. Good enough, was my
> thinking.
>
> As far as all the flushing and so on, that's what Jim had for his
> ``write`` helper function, so I suspect there's a point there too. It
> could probably be somewhat refactored but...it's a test harness.

Simply add a docstring explaining that we use this helper to update files to
make sure that the mtime of the file is updated so that buildout pick up the
changes, with a note that the resolution of mtime is system dependant. (And
possibly rename it to update_file() or something like that?)

And instead of busy-waiting, why don't you add a time.sleep(0.5) in there?

Cheers

  review approve

--
Francis J. Lacoste
<email address hidden>

On Jul 14, 2009, at 9:27 AM, Francis J. Lacoste wrote:

> Review: Approve
> On July 13, 2009, Gary Poster wrote:
>> In general, the necessity is caused by the mtime precision. On my
>> Mac, the precision is 1 second. Here's an example run of a slightly
>> modified version of the helper (adding a print), in the interactive
>> prompt.
>>
>>>>> def write_and_wait(dir, *args):
>> ... path = os.path.join(dir, *(args[:-1]))
>> ... original = os.stat(path).st_mtime
>> ... while os.stat(path).st_mtime == original:
>> ... print original
>> ... f = open(path, 'w')
>> ... f.write(args[-1])
>> ... f.flush()
>> ... os.fsync(f.fileno())
>> ... f.close()
>> ...
>>>>> f = open('/Users/gary/dev/deleteme', 'w')
>>>>> f.write('test')
>>>>> f.close()
>>>>> write_and_wait('/Users/gary/dev/deleteme', 'test')
>> 1247536625.0
>>>>> write_and_wait('/Users/gary/dev/deleteme', 'test')
>> 1247536627.0
>>>>> write_and_wait('/Users/gary/dev/deleteme', 'test')
>> 1247536628.0
>>>>> write_and_wait('/Users/gary/dev/deleteme', 'test')
>> 1247536629.0
>> 1247536629.0
>> 1247536629.0
>> [...there are 299 of these...]
>>>>>
>>
>> So, the precision is the point. I tried a variety of things to
>> trigger the mtime, but writing an empty string didn't work, and
>> directly writing the mtime is OS-specific/dependent, and I didn't
>> really want to go there. This is a test harness. Good enough, was
>> my
>> thinking.
>>
>> As far as all the flushing and so on, that's what Jim had for his
>> ``write`` helper function, so I suspect there's a point there too.
>> It
>> could probably be somewhat refactored but...it's a test harness.
>
> Simply add a docstring explaining that we use this helper to update
> files to
> make sure that the mtime of the file is updated so that buildout
> pick up the
> changes, with a note that the resolution of mtime is system
> dependant. (And
> possibly rename it to update_file() or something like that?)
>
> And instead of busy-waiting, why don't you add a time.sleep(0.5) in
> there?

OK, cool, I like the name change in particular.

I actually left out the time.sleep as a policy-ish decision, wanting
the tests to go as fast as possible, but it was an experiment, and I'm
happy to switch back to the time.sleep approach.

Incremental diff below.

Thanks

Gary

Index: z3c/recipe/filetemplate/tests.py
===================================================================
--- z3c/recipe/filetemplate/tests.py (revision 101900)
+++ z3c/recipe/filetemplate/tests.py (revision 101901)
@@ -13,14 +13,21 @@

##############################################################################

  import os
+import time
  import zc.buildout.testing
  import zc.buildout.tests
  from zope.testing import doctest

-def write_and_wait(dir, *args):
+def update_file(dir, *args):
+ """Update a file.
+
+ Make sure that the mtime of the file is updated so that buildout
notices
+ the changes. The resolution of mtime is system dependent, so we
keep
+ trying to write until mtime has actually changed."""
      path = os.path.join(dir, *(args[:-1]))
      original = os.stat(path).st_mtime
      wh...

On July 14, 2009, Gary Poster wrote:
> I actually left out the time.sleep as a policy-ish decision, wanting
> the tests to go as fast as possible, but it was an experiment, and I'm
> happy to switch back to the time.sleep approach.
>

Right, but know that busy waiting is bad in general because it makes it harder
for the OS to schedule other task (since you are competing for the CPU).

>
> Index: z3c/recipe/filetemplate/tests.py
> ===================================================================
> --- z3c/recipe/filetemplate/tests.py (revision 101900)
> +++ z3c/recipe/filetemplate/tests.py (revision 101901)
> @@ -13,14 +13,21 @@
>
> ###########################################################################
>###
>
> import os
> +import time
> import zc.buildout.testing
> import zc.buildout.tests
> from zope.testing import doctest
>
> -def write_and_wait(dir, *args):
> +def update_file(dir, *args):
> + """Update a file.
> +
> + Make sure that the mtime of the file is updated so that buildout
> notices
> + the changes. The resolution of mtime is system dependent, so we
> keep
> + trying to write until mtime has actually changed."""
> path = os.path.join(dir, *(args[:-1]))
> original = os.stat(path).st_mtime
> while os.stat(path).st_mtime == original:
> + time.sleep(0.2)
> f = open(path, 'w')
> f.write(args[-1])
> f.flush()

If you want the test to run as fast as possible I suggest:

while True:
  # Write the change
  # ...
  if os.stat(path).st_mtime != original:
   break
  time.sleep(0.2)

Use it if you like it.

--
Francis J. Lacoste
<email address hidden>

On Jul 14, 2009, at 10:09 AM, Francis J. Lacoste wrote:

> On July 14, 2009, Gary Poster wrote:
>> I actually left out the time.sleep as a policy-ish decision, wanting
>> the tests to go as fast as possible, but it was an experiment, and
>> I'm
>> happy to switch back to the time.sleep approach.
>>
>
> Right, but know that busy waiting is bad in general because it makes
> it harder
> for the OS to schedule other task (since you are competing for the
> CPU).

Bah, get more cores. :-)

>> Index: z3c/recipe/filetemplate/tests.py
>> ===================================================================
>> --- z3c/recipe/filetemplate/tests.py (revision 101900)
>> +++ z3c/recipe/filetemplate/tests.py (revision 101901)
>> @@ -13,14 +13,21 @@
>>
>> ###########################################################################
>> ###
>>
>> import os
>> +import time
>> import zc.buildout.testing
>> import zc.buildout.tests
>> from zope.testing import doctest
>>
>> -def write_and_wait(dir, *args):
>> +def update_file(dir, *args):
>> + """Update a file.
>> +
>> + Make sure that the mtime of the file is updated so that buildout
>> notices
>> + the changes. The resolution of mtime is system dependent, so we
>> keep
>> + trying to write until mtime has actually changed."""
>> path = os.path.join(dir, *(args[:-1]))
>> original = os.stat(path).st_mtime
>> while os.stat(path).st_mtime == original:
>> + time.sleep(0.2)
>> f = open(path, 'w')
>> f.write(args[-1])
>> f.flush()
>
>
> If you want the test to run as fast as possible I suggest:
>
> while True:
> # Write the change
> # ...
> if os.stat(path).st_mtime != original:
> break
> time.sleep(0.2)
>
> Use it if you like it.

Sure, cool. Done.

Thanks

Gary