Merge lp://staging/~gary/z3c.recipe.filetemplate/relative-paths into lp://staging/z3c.recipe.filetemplate

For reference, this is a patch to Launchpad that takes advantage of this new feature.

http://pastebin.ubuntu.com/418823/

After this patch and lp~gary/zc.buildout/python-support-9-relative-paths, absolute paths based on the build are only found in Launchpad in three locations:

- in scripts generated by z3c.recipe.tag (would be easy to fix)
- in scripts generated by z3c.recipe.i18n (would be easy to fix)
- in code generated by our Mailman integration (would be hard to fix, but after we move to Python 2.6 Barry Warsaw may be willing and able to help us move to the new Mailman code base).

I have a couple of minor comments and questions. Should be good to go.

> === modified file 'z3c/recipe/filetemplate/README.txt'

> + >>> cat(sample_buildout, 'bin', 'dosomething.sh') # doctest: +ELLIPSIS
> + #!/bin/sh
> + Z3C_RECIPE_FILETEMPLATE_BASE=`\
> + readlink -f "$0" 2>/dev/null || \
> + realpath "$0" 2>/dev/null || \
> + type -P "$0" 2>/dev/null`
> + Z3C_RECIPE_FILETEMPLATE_BASE=`dirname ${Z3C_RECIPE_FILETEMPLATE_BASE}`
> + Z3C_RECIPE_FILETEMPLATE_BASE=`dirname ${Z3C_RECIPE_FILETEMPLATE_BASE}`
> + cat "$Z3C_RECIPE_FILETEMPLATE_BASE"/data/info.csv
> +

The multiple mutation of Z3C_RECIPE_FILETEMPLATE_BASE are confusing.
Especially, the two identical Z3C_RECIPE_FILETEMPLATE_BASE=`dirname ...`
I assume that's the way to get to the parent directory of the script. Would be
clearer to use multiple variables, at the detriment of more namespace
polluting. At the very least a comment would help the reader. Your call.

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

> + def _get(self, section, option, start):
> + if section is None:
> + section = self.recipe.name # This sets up error messages properly.
> + if section == self.recipe.name:
> + factory = self.recipe.dynamic_options.get(option)
> + if factory is not None:
> + try:
> + return factory(self, start, option)
> + except:
> + # Argh. Would like to raise wrapped exception.
> + colno, lineno = self.get_colno_lineno(start)
> + msg = ('Dynamic option %r in line %d, col %d of %s '
> + 'crashed.') % (option, lineno, colno, self.source)
> + self.recipe.logger.error(msg, exc_info=True)
> + raise
> + # else...
> + options = self.recipe.options
>

Shouldn't you catch and re-raise SystemExit and KeyboardInterrupt? Or was this
fixed in Python 2.5?
> - def substitute(self, recipe, seen):
> + def substitute(self):
> def convert(mo):
> + start = mo.start()
> # Check the most common path first.
> - option = mo.group('braced_single')
> + option = mo.group('option')
> if option is not None:
> - val = self._get(recipe.options, recipe.name, option, seen,
> - mo.start('braced_single'))
> + section = mo.group('section')
> + val = self._get(section, option, start)
> + path_extension = mo.group('path_extension')
> + filters = mo.group('filters')
> + if path_extension is not None:
> + val = os.path.join(val, *path_extension.split('/')[1:])
> + if filters is not None:
> + for filter in filters.split('|')[1:]:
> + filter = filter.strip()
> + if filter not in self.recipe.filters:
> + colno, lineno = self.get_colno_lineno(start)
> + raise ValueError(
> + '...

On April 19, 2010, Gary Poster wrote:
> For reference, this is a patch to Launchpad that takes advantage of this
> new feature.
>
> http://pastebin.ubuntu.com/418823/
>
> After this patch and lp~gary/zc.buildout/python-support-9-relative-paths,
> absolute paths based on the build are only found in Launchpad in three
> locations:
>
> - in scripts generated by z3c.recipe.tag (would be easy to fix)

We don't care about that one in deployment.

> - in scripts generated by z3c.recipe.i18n (would be easy to fix)

Nor about this one.

> - in code generated by our Mailman integration (would be hard to fix, but
> after we move to Python 2.6 Barry Warsaw may be willing and able to help
> us move to the new Mailman code base).

That one means that we'll probably still need to run make clean on the mailman
machine at least?

--
Francis J. Lacoste
<email address hidden>

On Apr 19, 2010, at 4:28 PM, Francis J. Lacoste wrote:

> Review: Approve
> I have a couple of minor comments and questions. Should be good to go.
>
>> === modified file 'z3c/recipe/filetemplate/README.txt'
>
>> + >>> cat(sample_buildout, 'bin', 'dosomething.sh') # doctest: +ELLIPSIS
>> + #!/bin/sh
>> + Z3C_RECIPE_FILETEMPLATE_BASE=`\
>> + readlink -f "$0" 2>/dev/null || \
>> + realpath "$0" 2>/dev/null || \
>> + type -P "$0" 2>/dev/null`
>> + Z3C_RECIPE_FILETEMPLATE_BASE=`dirname ${Z3C_RECIPE_FILETEMPLATE_BASE}`
>> + Z3C_RECIPE_FILETEMPLATE_BASE=`dirname ${Z3C_RECIPE_FILETEMPLATE_BASE}`
>> + cat "$Z3C_RECIPE_FILETEMPLATE_BASE"/data/info.csv
>> +
>
> The multiple mutation of Z3C_RECIPE_FILETEMPLATE_BASE are confusing.
> Especially, the two identical Z3C_RECIPE_FILETEMPLATE_BASE=`dirname ...`
> I assume that's the way to get to the parent directory of the script. Would be
> clearer to use multiple variables, at the detriment of more namespace
> polluting. At the very least a comment would help the reader. Your call.

Done, with variable name changes and comments, for both shell and Python versions.

>
>> === modified file 'z3c/recipe/filetemplate/__init__.py'
>
>> + def _get(self, section, option, start):
>> + if section is None:
>> + section = self.recipe.name # This sets up error messages properly.
>> + if section == self.recipe.name:
>> + factory = self.recipe.dynamic_options.get(option)
>> + if factory is not None:
>> + try:
>> + return factory(self, start, option)
>> + except:
>> + # Argh. Would like to raise wrapped exception.
>> + colno, lineno = self.get_colno_lineno(start)
>> + msg = ('Dynamic option %r in line %d, col %d of %s '
>> + 'crashed.') % (option, lineno, colno, self.source)
>> + self.recipe.logger.error(msg, exc_info=True)
>> + raise
>> + # else...
>> + options = self.recipe.options
>>
>
> Shouldn't you catch and re-raise SystemExit and KeyboardInterrupt? Or was this
> fixed in Python 2.5?

Changed.

>> - def substitute(self, recipe, seen):
>> + def substitute(self):
>> def convert(mo):
>> + start = mo.start()
>> # Check the most common path first.
>> - option = mo.group('braced_single')
>> + option = mo.group('option')
>> if option is not None:
>> - val = self._get(recipe.options, recipe.name, option, seen,
>> - mo.start('braced_single'))
>> + section = mo.group('section')
>> + val = self._get(section, option, start)
>> + path_extension = mo.group('path_extension')
>> + filters = mo.group('filters')
>> + if path_extension is not None:
>> + val = os.path.join(val, *path_extension.split('/')[1:])
>> + if filters is not None:
>> + for filter in filters.split('|')[1:]:
>> + filte...