backslashes in windows paths should be doubled

Bug #358719 reported by Jeroen Michiel
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
grok
Fix Released
High
Uli Fouquet

Bug Description

If installing Grok 1.0a2 on windows, several paths are stored (e.g. deploy.ini, debug.ini) that contain single backslashes, like D:\grok1.0a2\arts/parts/log/access.log

This causes trouble, as \ can also be the escape character in strings. Here \a might be replaced by 0x07 (or the 'Bell' control character).

It would be best to either double the backslashes, or to use slashes.

Revision history for this message
Ben Dadsetan (bdadsetan) wrote :

This was reported multiple times on the mailing list.
To reproduce, the parameter to grokproject MUST start either an 'a', 't' or another character that is translated after a back-slash ('\').

An easy (and very ugly) work-around:
start the project name with a 'z' or a 'g'.

Potential solutions:
1) One solution is to follow zopeproject's methodology. Use relative paths for log and Data.fs rather than full paths in zope.conf.in.
See patch attached.

I do not have any check-in rights, so please apply patch to grokproject code if you agree with the solution.

Downside:
This forces developers to run paster out of the "root" project directory rather than anywhere they like. I am not sure this is not the case anyway right now, but if not, we are making it mandatory again.

2) Make zc.buildout always translate $${buildout:directory} to a forward slash path

3) Make zc.buildout always "double" the backslash ('\') characters to escape them.

As I am not too familar with buildout and all the development tools used by grok/zope3 I am not really sure how:
a) To build automated tests for my patch. If someone is willing to give me advice, I will build tests.
b) To get buildout to regenerate parts/etc/* from etc/*_in. So I could only test changing the parts/etc/* files..

Revision history for this message
Ben Dadsetan (bdadsetan) wrote :

Patch and suggestions made. Developer with SVN access needs to make a decision.

Changed in grok:
status: New → In Progress
Revision history for this message
Uli Fouquet (uli-gnufix) wrote :

Hi Ben,

thanks for the bug report!

One question: what happens, if you change the paths in

   D:\grok1.0a2\arts\etc\*.ini files

to contain only slashes _or_ backslashes (not both)?

Both cases could be interesting.

Revision history for this message
Uli Fouquet (uli-gnufix) wrote :

Oh, this bug report is from Jereon. Sorry, Jereon!

Maybe you could also tell something about my last question?

To correct myself: you might try to change all paths in *.in files in the etc/ subdirectory of your project.

Then rerun buildout and see whether everything works as expected.

Maybe we can fix the problem by patching grokprojects template generation process.

Revision history for this message
Ben Dadsetan (bdadsetan) wrote :

Ok.. I think my original fix was OK but not very nicely sold as it looked rather like a work-around than a fix. Relative paths are in my book a reasonable way to go but I also dislike the concept of being forced to run paster from a specific directory.

Here is another fix that solves the real issue. $${buildout:directory} gets translated differently depending on the platform buildout is run on (it uses os.path functions). This is reasonable but it then looks very ugly when mixed with "hard-coded platform path" such as $${buildout:directory}/parts/logs/access.log which becomes C:\grok\art/parts/logs/access.log
It appears ugly but works just fine.

Our problem is when we pass a string to a handler though ConfigParser ini files. This happens in both etc/deploy.ini_in_tmpl and etc/debug.ini_in_tmpl when we give our handler_accesslog a string and ::func::open() access mode. The string is then "interpreted" by python before it is fed to the ::func::open() method. All backslashes are translated at runtime. When the backslash is followed by a character without known translation the backslash is automatically quoted. \g becomes \\g but \a become \0x10

My newly attached patch resolves potential interpretation issues by calling the ::func::open() with a raw string. This way any translation of $${buildout:directory} is not "re-interpreted" by python. I am not sure it handles unicode paths, but I guess it does not have to just yet.

To answer your questions: Yes hard-coding \\ instead of \ helps and yes using / instead of \ helps.

See my patch to deploy.ini_in_tmpl and debug.ini_in_tmpl

Revision history for this message
Ben Dadsetan (bdadsetan) wrote :

By the way, I tested my patch with a clean environment with all other changes available on the SVN trunk. It now goes smoothly for me on Windows.

Other serious bugs were also corrected on the trunk.

If we have no futher intentions for grokproject 1.0a4 I suggest we make a release soon with my patch or with another fix for this bug.

Revision history for this message
Roger Erens (rogererens) wrote :

An alternative may be to use os.path.join to join the path $${buildout:directory} and the path parts/logs/access.log?

Remember to test the patch also on other platforms like Linux and MacOS X.

Revision history for this message
Ben Dadsetan (bdadsetan) wrote :

Hi Roger,

  Fair point. It is dangerous to assume anything. I have now tested on all 3 platforms (MacOS 10.5, Latest Ubuntu and Windows XP). Things are looking good. I just hope I have not made any assumptions while testing and I sure wish there was more automated testing in there. I am not familar enough with any of the related technologies to add automated tests at this stage.

  Concerning your alternative, it still would not resolve our problem. It would produce "nicer to look at" debug.ini and deploy.ini files, but the backslash problem would still be there. The windows platform os.path.join joins strings with backslashes that are then reinterpreted in python before calling the handler_accesslog.

  My perspective: We can use os.path.join, but this would require more code however it would result in less "ugly" output. Using os.path.join will not resolve this bug.
Do you wish to contribute such changes as a feature enhancement unrelated to this bug?

Note my tests included for each platform:
  - Easy_installing the SVN trunk + my patch version of a grokproject egg onto a freshly installed python (windows, ubuntu) or on a fresh virtualenv --no-site-packages (Mac OS)
  - Running the resulting grokproject
  - Running paster on the generated parts/etc/deploy.ini
  - Creating the delivered app.py into the ZODB root
  - Viewing the newly installed app's URL

Revision history for this message
Uli Fouquet (uli-gnufix) wrote :

Hi Ben and everybody,

First many thanks for the analysis and patches! The 'raw-string' trick was surprising to me but might be very helpful indeed.

Do I understand it correctly, that with your patch applied something like

  C:\\path\\to\\proj/parts/etc/zope.conf

is generated? I'd still consider this a broken path name although it might be handled correctly by the different parsers used. But before going more into detail I'd like to make sure this is really the case.

I also think that Roger's proposal is basically the right way to go. If we had to quote any backslashes afterwards, we could simply do that.

Unfortunately I still have not a working Windows env running, so when I check in a patch, could someone try to run the modified grokproject trunk on a Win-based platform (i.e. run the modified grokproject)? I'd be glad to help with instructions how to do this.

I am pretty sure, that the automated tests will miserably fail on Windows for various path problems of similar kind, but this is another (big) topic.

Revision history for this message
Uli Fouquet (uli-gnufix) wrote :

Forgot to set the importance: this bug is a showstopper on windows.

Changed in grok:
assignee: nobody → uli-gnufix
importance: Undecided → High
milestone: none → 1.0
Revision history for this message
Ben Dadsetan (bdadsetan) wrote :

Hi Uli,

   Yes you are right. It would in effect "automatically quote" any backslashes.
If $${buildout:directory} is C:\grok\art,
r'$${buildout:directory}/parts/log/access.log'
  would become
r'C:\grok\art/parts/log/access.log'
Which is the equivalent of
'C:\\grok\\art/parts/log/access.log'

The advantage of using a raw string is it would probably also be happy to accept strange international characters (I can not test that with my U.S. PC) just in the same way all the other areas of buildout accept these when they are not passed as "strings".

Using os.path.join all the way on windows would produce
r'C:\\grok\\art\\parts\\log\\access.log' with my fix
Without my patch, using os.path.join would produce
'C:\grok\art\parts\log\access.log' which would give us the same problem again as this would be the equivalent of:
'C:\\grok\0x10rt\\parts\\log\0x10ccess.log' which of course can not be opened.

I don't know how to use os.path.join all the way as I am not enough familiar with buildout. Would we have to create an entry like the unused package_directory variable in templates.py?

Note that the strings with mixed backslashes and forward slashes are all over the place (even in generated comments). To be consistent, if you want to remove all mixes, you will have to use os.path.join (or a '/'.join) for many different paths. It could be 5-6 new variables in templates.py (would be the only way I, the newbie, knows).

My personal conclusion:
I prefer something consistent (either all backslashes [windows] or all forward slashes [all platforms including windows]) because it does look nicer.
The downside I see is more code in grokproject and more unspoken rules to develop grokproject. We already have a "dead" variable '''project_directory''' which is no longer used anywhere and with time I imagine us having plenty of dead variables.

I do not consider mixed slashes "broken" because if it works it ain't broken :)

Revision history for this message
Uli Fouquet (uli-gnufix) wrote : Re: [Bug 358719] Re: backslashes in windows paths should be doubled
Download full text (4.5 KiB)

Hi Ben,

thanks again for all the investigations!

Ben Dadsetan wrote:

> Yes you are right. It would in effect "automatically quote" any backslashes.
> If $${buildout:directory} is C:\grok\art,
> r'$${buildout:directory}/parts/log/access.log'
> would become
> r'C:\grok\art/parts/log/access.log'
> Which is the equivalent of
> 'C:\\grok\\art/parts/log/access.log'
>
> The advantage of using a raw string is it would probably also be happy
> to accept strange international characters (I can not test that with my
> U.S. PC) just in the same way all the other areas of buildout accept
> these when they are not passed as "strings".

I am not at all against using raw strings here, I'd only prefer path
names built in one style.

> Using os.path.join all the way on windows would produce
> r'C:\\grok\\art\\parts\\log\\access.log' with my fix Without my patch,
> using os.path.join would produce 'C:\grok\art\parts\log\access.log'
> which would give us the same problem again as this would be the
> equivalent of: 'C:\\grok\0x10rt\\parts\\log\0x10ccess.log' which of
> course can not be opened.

So os.path.join(r'${buildout:directory}', 'baz', 'bar') should give us
the right thing?

> I don't know how to use os.path.join all the way as I am not enough
> familiar with buildout.

os.path is part of standard python, as you know :) We need a solution,
that one can apply by just modifying etc/deploy.ini.in and running
buildout afterwards. Buildout will not do more than just replacing the
string ${buildout-directory} by the string C:\\foo\bar or similar.

To be more precise: it is the z3c.recipe.template recipe that performs
this substitution.

> Would we have to create an entry like the unused
> package_directory variable in templates.py?

Oh, we could, but I am not sure, that this would help. As said, we're
looking for a solution that can be applied to existing projects with
latest grokproject/grok. If adding an appropriate variable would help,
then this would be fine.

Maybe we could even use the ``package_directory`` variable, just renamed
to ``posix_projectdir``. If not, we should remove it. Thanks for
spotting this!

> Note that the strings with mixed backslashes and forward slashes are all
> over the place (even in generated comments). To be consistent, if you
> want to remove all mixes, you will have to use os.path.join (or a
> '/'.join) for many different paths. It could be 5-6 new variables in
> templates.py (would be the only way I, the newbie, knows).

Right, that would not be a solution, just a mess ;) Instead we could do
smarter substitutions in z3c.recipe.template. This is the piece of code
that actually generates the config files from the templates in etc/. And
it is rather straight.

>From what I see, one variable might be enough, that holds the
posix-compatible form of ${buildout:directory}. We could call it
``posix-buildoutdir`` and then give paths like this::

  ${posix-buildoutdir}/parts/etc/zope.conf

resulting in::

  C://my/path/to/project/parts/etc/zope.conf

Anyway, it would be nice to collect all 'broken' paths. Could you place
a set of generated config files (in parts/etc/) and their templates (in
etc/) somewhere? Or only p...

Read more...

Revision history for this message
Ben Dadsetan (bdadsetan) wrote :

Hi!

  I played quite a bit around and as I expected, os.path is of course available in python but not generally in our in_tmpl files. These are plain text files that are parsed by our software but generally do not contain python. Your option 2 would still work though.

  Anyway, before I prepare another patch for testing, I wanted to get your thoughts on the following. Let me give you the run-down of your alternatives and then let us choose one.

1) posixish paths all the way (posix_projectdir)
  Note posix does not have the concept of a disk drive (C:). Starting a posix path with a 'letter' and ':' makes it very much look like a URL with a strange protocol. Or even like ssh/scp host:/path/to/somewhere format. Technically this should not matter too much until the day the python open() accepts URLs or scp/rcp paths :)

  I have played with this quite a bit and even Windows UNCs work:
C:/my/path is properly interpreted as C:\my\path
//mint.blue/share is interpreted as \\mint.blue\share

  So this works and looks better. It does not follow any standards though (no it is not posix).

2 and 3) These would leave plenty of mixed paths behind, which is quite ugly and I thought still is a goal. I would know how to prepare a patch for 2 but not 3.

There is 4 and 5 as well :)
4) A new template variable to separate paths (for example: $${os_sep})
  This makes the template files look quite ugly but keeps the python code clean of crazy stuff.
We would change $${buildout:directory}/parts/etc/zope.conf to $${buildout:directory}$${os_sep}parts$${os_sep}etc$${os_sep}zope.conf

  Just an idea really...

Finally there is my new favorite option:
!!!5!!!
5) Let us create one variable for each target directory (one for parts/etc one for parts/log, ...) and let us make this variable's value end with the platform specific separator.

So $${buildout:directory}/parts/etc/zope.conf would become ${target_etc}zope.conf

Advantages:
- Uli, you are currently getting feedback on path locations. This change would allow us to make a switch to any new format by changing only 4 lines in templates.py.
- Also it follows what the user of a platform knows. So Windows users will see their ugly \ and other users will see neat /

Disadvantage:
- It looks a little ugly in the template but I think it is acceptable.
- It also requires to pass a raw string

I think people really want a new release of grokproject soon. So let us choose one of our 5 possibilities.
I am happy to supply a patch for any of 1, 2, 4, and 5, if it helps you. In any case, I volunteer for testing on my 3 platforms (MacOS, Ubuntu, XP)

PS: I am happy to contibute to grok even if it is such tiny patches. Even if I do not use Windows every day I feel it is important to support it well to attract a larger user base.

Revision history for this message
Uli Fouquet (uli-gnufix) wrote :
Download full text (5.3 KiB)

Hi Ben,

> I played quite a bit around and as I expected, os.path is of course
> available in python but not generally in our in_tmpl files. These are
> plain text files that are parsed by our software but generally do not
> contain python. Your option 2 would still work though.

I assume you're talking about the Python expression in deploy.conf.in?
Yes, the config files are pure plain text but this specific expression
is evaluated by paster (please do not ask where exactly ;).

I checked on Ubuntu, where::

args = (os.path.join(r'${buildout:directory}', 'parts', 'log', \
        'access.log'), 'a')

is interpreted correctly. Looks like the `os` module is available when
the expression gets evaluated.

Can you confirm that behaviour for Windows too?

> Anyway, before I prepare another patch for testing, I wanted to get
> your thoughts on the following. Let me give you the run-down of your
> alternatives and then let us choose one.
>
> 1) posixish paths all the way (posix_projectdir) Note posix does not
> have the concept of a disk drive (C:). Starting a posix path with a
> 'letter' and ':' makes it very much look like a URL with a strange
> protocol. Or even like ssh/scp host:/path/to/somewhere format.
> Technically this should not matter too much until the day the python
> open() accepts URLs or scp/rcp paths :)

Right. To be more standard-conform, one could use the urlparse functions
from urllib/urllib2. But this (generates URLs like 'file:///foo/bar')
would not be accepted by regular `open()` I think.

> I have played with this quite a bit and even Windows UNCs work:
> C:/my/path is properly interpreted as C:\my\path
> //mint.blue/share is interpreted as \\mint.blue\share

Nice. Good to know.

> So this works and looks better. It does not follow any standards
> though (no it is not posix).
>
> 2 and 3) These would leave plenty of mixed paths behind, which is quite
> ugly and I thought still is a goal.

Yeah, let's try to use only one path separator.

> I would know how to prepare a patch
> for 2 but not 3.
>
> There is 4 and 5 as well :)
> 4) A new template variable to separate paths (for example: $${os_sep})
> This makes the template files look quite ugly but keeps the python code clean of crazy stuff.
> We would change $${buildout:directory}/parts/etc/zope.conf to $${buildout:directory}$${os_sep}parts$${os_sep}etc$${os_sep}zope.conf

Hm, why not? Note, that in generated templates $${foo} becomes ${foo},
which is better readable. The additional leading '$' quotes the
following '${}' expression, so that buildout does not replace the whole
expression when generating the templates (took me some time to find that
out, therefore I carry this 'hard-earned wisdom' forward here ;).

> Just an idea really...
>
> Finally there is my new favorite option: !!!5!!!
>
> 5) Let us create one
> variable for each target directory (one for parts/etc one for
> parts/log, ...) and let us make this variable's value end with the
> platform specific separator.
>
> So $${buildout:directory}/parts/etc/zope.conf would become
> ${target_etc}zope.conf
>
> Advantages:
> - Uli, you are currently getting feedback on path
> locations. This ...

Read more...

Revision history for this message
Ben Dadsetan (bdadsetan) wrote :

Hi,

[snip]
> I checked on Ubuntu, where::
>
> args = (os.path.join(r'${buildout:directory}', 'parts', 'log', \
>        'access.log'), 'a')
>
> is interpreted correctly. Looks like the `os` module is available when
> the expression gets evaluated.
>
> Can you confirm that behaviour for Windows too?

I checked. It works fine.
args = (os.path.join(r'$${buildout:directory}','parts','log','access.log'),'a')
   translates to:
args = (os.path.join(r'C:\grok\abart','parts','log','access.log'),'a')

[snip]
> Right. To be more standard-conform, one could use the urlparse functions
> from urllib/urllib2. But this (generates URLs like 'file:///foo/bar')
> would not be accepted by regular `open()` I think.

Correct, it would not be accepted and I think it is way overkill a
reengineering. If we do get the usecase sometime we might want to do
it but being able to manage writes to any URL is quite a task :)

[snip]
> Yes, we must release soon, as currently grokproject generates broken
> projects.
>
> Therefore I am currently tending to opt for one of your first patches
> (passing ${buildout:directory} as raw string), maybe enriched with
> `os.path.join()` if that works on Windows.

I have tested this as I explained above and it works. For now let us
go for this. We will have mixed paths but it will allow us to build
projects with grokproject again. By the way, the little experience I
had with Cygwin (somewhat unrelated but still has path experiece in
windows world) showed there is often mixed paths on such systems. With
pure shell scripts it can even break things. But as the CPython
interpreter for Windows accept it I feel we can live with it for now.

>
> This is a change which is good to handle. Could you anyway give a list
> of other paths generated on Windows in the different config files? What
> for instance becomes of the %(here)s/zope.conf expression in
> deploy.ini.in? What looks the `site-definition` on top of zope.conf
> looks like in generated zope.conf? Do you get a better formed path when
> in zope.conf.in you replace::
>
>    path ${buildout:directory}/parts/data/Data.fs
>
> (in the zodb/filestorage section) with::
>
>    path ${data:path}
>
> and rerun buildout on Windows?

path ${data:path} becomes a neat:
path C:\grok\adart\parts\data\Data.fs

Revision history for this message
Uli Fouquet (uli-gnufix) wrote :

Hi,

> Hi,
>
> [snip]
> > I checked on Ubuntu, where::
> >
> > args = (os.path.join(r'${buildout:directory}', 'parts', 'log', \
> > 'access.log'), 'a')
> >
> > is interpreted correctly. Looks like the `os` module is available when
> > the expression gets evaluated.
> >
> > Can you confirm that behaviour for Windows too?
>
> I checked. It works fine.

Excellent :)

> args = (os.path.join(r'$${buildout:directory}','parts','log','access.log'),'a')
> translates to:
> args = (os.path.join(r'C:\grok\abart','parts','log','access.log'),'a')

And this expression gives what when run on a Windows Python shell? Here
on Ubuntu it gives as expected:

  >>> import os
  >>> os.path.join(r'C:\grok\abart', 'foo', 'bar.baz')
  'C:\\grok\\abart/foo/bar.baz'

and should be::

  'C:\\grok\\abart\\foo\\bar.baz'

on Windows, right?

Well, if you can start an instance without problems with that setting,
then we're fine off.

[snip: urllib for paths]
>
> Correct, it would not be accepted and I think it is way overkill a
> reengineering. If we do get the usecase sometime we might want to do
> it but being able to manage writes to any URL is quite a task :)

Yep, nice to have but overkill for now :)

> > Yes, we must release soon, as currently grokproject generates broken
> > projects.
> >
> > Therefore I am currently tending to opt for one of your first patches
> > (passing ${buildout:directory} as raw string), maybe enriched with
> > `os.path.join()` if that works on Windows.
>
> I have tested this as I explained above and it works. For now let us
> go for this. We will have mixed paths but it will allow us to build
> projects with grokproject again. By the way, the little experience I
> had with Cygwin (somewhat unrelated but still has path experiece in
> windows world) showed there is often mixed paths on such systems. With
> pure shell scripts it can even break things. But as the CPython
> interpreter for Windows accept it I feel we can live with it for now.

Interesting. So let's go with it :)

[snip]

> path ${data:path} becomes a neat:
> path C:\grok\adart\parts\data\Data.fs

Nice! :) Apparently there is room for further improvements in the config
file generation process. For example the default 'site-definition' in
zope.conf is wrong!

I will check in your patches later today and add some basic tests.

Revision history for this message
Roger Erens (rogererens) wrote :
  • p2 Edit (981 bytes, text/plain)

Hi Uli,

just to give you the idea about another choice: here's a patch for
z3c.recipe.template-0.1-py2.5.egg/z3c/recipe/template/__init__.py

When a backward slash is detected on a line, it is assumed that the line contains a Windows-style path.
The path is normalized, such that forward slashes are replace by backward slashes. To obtain strings in which the backslashes are escaped, the repr() is used. The function repr() returns strings that are single or double quoted; these quotes are trimmed, otherwise the processing of zope.conf by paster fails.

It results in paths on Windows having double backslashes (but not in the %(here)/zope.conf result). Linux paths still have the same forward slashes.

Cheers,

Roger

Revision history for this message
Uli Fouquet (uli-gnufix) wrote :

Hi Roger,

> just to give you the idea about another choice: here's a patch for
> z3c.recipe.template-0.1-py2.5.egg/z3c/recipe/template/__init__.py
>
> When a backward slash is detected on a line, it is assumed that the
> line contains a Windows-style path. The path is normalized, such that
> forward slashes are replace by backward slashes. To obtain strings in
> which the backslashes are escaped, the repr() is used. The function
> repr() returns strings that are single or double quoted; these quotes
> are trimmed, otherwise the processing of zope.conf by paster fails.
>
> It results in paths on Windows having double backslashes (but not in the
> %(here)/zope.conf result). Linux paths still have the same forward
> slashes.

Thanks for the patch! This is very similar to what I also thought about.
For now, however, I think we go better with Ben's already planned
solution. The reason is, that it looks a bit 'hidden' if
z3c.recipe.template starts changing the content it generates beyond
simple variable substitution. Right now it works quite straight: pull
something in, replace any ${}-vars, put it out.

Furthermore I'd like to avoid introducing new versions of dependent
packages (like z.r.template) at this point as this easily becomes an
upgrade mess and we currently have already enough of that problems ;)

In the long run, however, we might add such a functionality (say:
path-normalization) as an optional extra-feature. But IMHO it then
should really be an option that one can turn on/off explicitly in
buildout.cfg like this::

  [zope_conf]
  input = my/input/path
  output = my/output/path
  normalize_paths = 1

I am happy anyway, that you digged deeper into that! At least now we
know, that using this approach we can really do better path generation.

Hope, you understand if we go with the other solution for the upcoming
release, but we might use more advanced template parsing (that in fact
should happen in z3c.recipe.template then) in one of the next releases
of z3c.repice.template/grokproject.

Best regards,

Uli

Revision history for this message
Roger Erens (rogererens) wrote :

Hi Uli,

fine with me to use Ben's solution, no problem at all! For me this was also very much a coding experience: it's only now that I have overcome my fear of digging into code in eggs :-). And I finally understand which files and variables Ben and you were referring to.

The only pitfall I see is that more and more Python-constructs are getting used in the .ini files. We then might end up just as well as doing the configuration in Python files. (Wasn't that the type of argument used to introduce ZCML?)

Maybe yet another solution for the long run might be to not use strings altogether in the [handler_accesslog] section and just write
log_name=${buildout:directory}/parts/log/access.log
mode=a

After all, you're also not using e.g.
logger_name = 'wsgi'
(mind the quotes) in the [filter:translogger] section.

Good luck with patching; I hope I can soon remove the work-around side-bars in the installation HowTo!

Roger

Revision history for this message
Ben Dadsetan (bdadsetan) wrote :
Download full text (3.2 KiB)

Uli,
to answer your earlier question, on python 2.5.4 on windows:
>>> import os
>>> os.path.join(r'C:\grok\abart', 'foo', 'bar.baz')
'C:\\grok\\abart\\foo\\bar.baz'

Obviously, on linux, your $${buildout:directory} will produce forward
slashes so the entire path becomes forward slashes.

Roger,
I think that is an interesting place to make the correction. Also, I
did not know the magical os.path.normpath - seems to have been created
for our purpose :).

I do wonder why you chose to repr the result of os.path.normpath.
I get this:
>>> repr(os.path.normpath('args = ("c:\python25/license.txt", "a")')).strip('\'\"')
'args = ("c:\\\\python25\\\\license.txt", "a")'

without repr (and corresponding strip):
>>> os.path.normpath('args = ("c:\\python25/license.txt", "a")')
'args = ("c:\\python25\\license.txt", "a")'

You might have noticed with the repr, we get too many backslashes
whereas without we properly only have one backslash (that is quoted).

The other thing that would be preferable is only to quote the
backslashes when the path is inside quotes (case of our bug). When it
is just lying around the config file it should only have one
non-quoted backslash. Do you think we can resolve that with regular
expressions or the like?

2009/4/17 Roger Erens <email address hidden>:
> Hi Uli,
>
> fine with me to use Ben's solution, no problem at all! For me this was
> also very much a coding experience: it's only now that I have overcome
> my fear of digging into code in eggs :-). And I finally understand which
> files and variables Ben and you were referring to.

Same for me, I overcame my fear of egg-digging. :)

> The only pitfall I see is that more and more Python-constructs are
> getting used in the .ini files. We then might end up just as well as
> doing the configuration in Python files. (Wasn't that the type of
> argument used to introduce ZCML?)
>
> Maybe yet another solution for the long run might be to not use strings altogether in the [handler_accesslog] section and just write
> log_name=${buildout:directory}/parts/log/access.log
> mode=a

This would resolve the difference in handling between template
variables within strings and template variables "outside of strings".
I thought about this but that is because of my mission-critical
maintenance experience. If I do not understand a piece of code
5803424% I avoid making any non obvious changes :) It probably makes
more sense with an open-source project to add proper automated tests
and let oneself go a little wild in redesign if deemed necessary.

> After all, you're also not using e.g.
> logger_name = 'wsgi'
> (mind the quotes) in the [filter:translogger] section.
>
> Good luck with patching; I hope I can soon remove the work-around side-
> bars in the installation HowTo!

Thanks a lot for the installation HowTo. I hope we can simplify the
installation story even more with some sort of installer. Am battling
with my 3 neurons to evaluate a proper solution. Right now there are
too many things people need to think about or look at manually. Way
too much for most people to be willing to try grok out. Luckily we
have a few couragous people out there who are still giving feedback
and helping make our...

Read more...

Revision history for this message
Roger Erens (rogererens) wrote :
Download full text (3.5 KiB)

on 16-4-2009 20:45 Ben Dadsetan wrote:
...
>
> Roger,
> I think that is an interesting place to make the correction. Also, I
> did not know the magical os.path.normpath - seems to have been created
> for our purpose :).
>
> I do wonder why you chose to repr the result of os.path.normpath.
> I get this:
>>>> repr(os.path.normpath('args = ("c:\python25/license.txt", "a")')).strip('\'\"')
> 'args = ("c:\\\\python25\\\\license.txt", "a")'
>
> without repr (and corresponding strip):
>>>> os.path.normpath('args = ("c:\\python25/license.txt", "a")')
> 'args = ("c:\\python25\\license.txt", "a")'

Argh! I ran my code experiments by starting up buildout. To get
feedback, I used print statements all over the place. Doing the same
interactively:

 >>> print os.path.normpath('args = ("c:\\python25/license.txt", "a")')
args = ("c:\python25\license.txt", "a")

which lead me to believe I needed repr() to obtain the double backslashes.

But wait: when I'm not using the repr() function, parts\etc\deploy.ini
will contain a string as output by the print statement.
Do you see an alternative solution?

>
> You might have noticed with the repr, we get too many backslashes
> whereas without we properly only have one backslash (that is quoted).

See above. Yes in the interpreter, no in the output file.
>
> The other thing that would be preferable is only to quote the
> backslashes when the path is inside quotes (case of our bug). When it
> is just lying around the config file it should only have one
> non-quoted backslash. Do you think we can resolve that with regular
> expressions or the like?

You mean like in a comment as on the first line of deploy.ini?
Well, first we need to resolve whether repr() is or is not a kludgy
approach.

>
>
> 2009/4/17 Roger Erens <email address hidden>:
>> Hi Uli,
>>
>> fine with me to use Ben's solution, no problem at all! For me this was
>> also very much a coding experience: it's only now that I have overcome
>> my fear of digging into code in eggs :-). And I finally understand which
>> files and variables Ben and you were referring to.
>
> Same for me, I overcame my fear of egg-digging. :)
>
>> The only pitfall I see is that more and more Python-constructs are
>> getting used in the .ini files. We then might end up just as well as
>> doing the configuration in Python files. (Wasn't that the type of
>> argument used to introduce ZCML?)
>>
>> Maybe yet another solution for the long run might be to not use strings altogether in the [handler_accesslog] section and just write
>> log_name=${buildout:directory}/parts/log/access.log
>> mode=a
>
> This would resolve the difference in handling between template
> variables within strings and template variables "outside of strings".

Well, I'll leave that to Uli to decide.

> I thought about this but that is because of my mission-critical
> maintenance experience. If I do not understand a piece of code
> 5803424% I avoid making any non obvious changes :)

May I add my maintenance experience: avoid making any non obvious
changes even if you _do_ understand the piece of code :-P

>
>> After all, you're also not using e.g.
>> logger_name = 'wsgi'
>> (mind the quotes) in the [fil...

Read more...

Revision history for this message
Uli Fouquet (uli-gnufix) wrote :

Hi Ben and Roger,

First fix is committed following several results of all the contributions here. Thanks a lot for that!
Basically I only changed the Python expressions in deploy.ini.in and debug.ini.in to use os.path.join()
and raw strings.

As already said, it is difficult to test those kind of problems/fixes, so no tests were added.

Could someone please check, whether the current trunk of grokproject generates
working projects on Windows?

On Ubuntu I check this normally like this:

- Check out the trunk:

  $ svn co svn://svn.zope.org/repos/main/grokproject/trunk grokproject

- Run bootstrap/buildout

  $ python2.4 bootstrap/bootstrap.py
  $ bin/buildout

- Generate a project:

  $ bin/grokproject asample
  ...

- Run the project

  $ asample/bin/paster serve asample/parts/etc/deploy.ini

  (which reminds me, that the path to deploy.ini is too long)

I assume that it should work similar on Windows. It would be great If someone could
check where it hangs now :)

Revision history for this message
Ben Dadsetan (bdadsetan) wrote :

Hi,

  I tested on Windows and Mac (as you already tested Ubuntu). For our
patches everything is looking good.

I did encounter other issues. But both have workarounds.
On Windows, rerunning grokproject makes things work. On Mac, manually
running easy_install PasteScript and then rerunning grokproject makes
things work.

I am not sure why these two problems occur and I do not know how to
properly investigate this. Can someone teach me the best way to look
at these problems?

*****Windows XP SP3******
While:
  Installing eggbasket.

An internal error occured due to a bug in either zc.buildout or in a
recipe being used:
Traceback (most recent call last):
  File "c:\docume~1\ben\locals~1\temp\tmpwahq5-\zc.buildout-1.2.1-py2.5.egg\zc\buildout\buildout.py",
line 1509, in main

  File "c:\docume~1\ben\locals~1\temp\tmpwahq5-\zc.buildout-1.2.1-py2.5.egg\zc\buildout\buildout.py",
line 473, in insta
ll
  File "c:\docume~1\ben\locals~1\temp\tmpwahq5-\zc.buildout-1.2.1-py2.5.egg\zc\buildout\buildout.py",
line 1091, in _cal
l
  File "c:\docume~1\ben\buildo~1\eggs\z3c.recipe.eggbasket-0.4.1-py2.5.egg\z3c\recipe\eggbasket\downloader.py",
line 85,
 in install
    os.unlink(temp_tarball_name)
WindowsError: [Error 32] The process cannot access the file because it
is being used by another process: 'c:\\docume~1\\
ben\\locals~1\\temp\\tmpj2p4jz'

******* MAC OS X 10.5.6 ******

$ grokproject --version
Traceback (most recent call last):
  File "/Users/sysupbda/testinggrokproject/1.04dev/bin/grokproject",
line 5, in <module>
    from pkg_resources import load_entry_point
  File "/Users/sysupbda/testinggrokproject/1.04dev/lib/python2.5/site-packages/setuptools-0.6c9-py2.5.egg/pkg_resources.py",
line 2562, in <module>
  File "/Users/sysupbda/testinggrokproject/1.04dev/lib/python2.5/site-packages/setuptools-0.6c9-py2.5.egg/pkg_resources.py",
line 626, in require
  File "/Users/sysupbda/testinggrokproject/1.04dev/lib/python2.5/site-packages/setuptools-0.6c9-py2.5.egg/pkg_resources.py",
line 524, in resolve
pkg_resources.DistributionNotFound: PasteScript>=1.6

By the way Uli,
A really great patch. Now debug.ini and deploy.ini are free of mixed paths.
Only zope.conf and zdaemon.conf have mixed paths now. We can try to
look at those once we got this release out of the way (maybe on a new
bug entry?)

Revision history for this message
Uli Fouquet (uli-gnufix) wrote :

We're making progress on Windows :)

Changed in grok:
status: In Progress → Fix Committed
Revision history for this message
Uli Fouquet (uli-gnufix) wrote :

Hey Ben,

> I tested on Windows and Mac (as you already tested Ubuntu). For our
> patches everything is looking good.

Great! So finally we get rid of at least one showstopper. And this time
even before the release ;) Thanks for checking!

> I did encounter other issues. But both have workarounds.
> On Windows, rerunning grokproject makes things work. On Mac, manually
> running easy_install PasteScript and then rerunning grokproject makes
> things work.
>
> I am not sure why these two problems occur and I do not know how to
> properly investigate this. Can someone teach me the best way to look
> at these problems?
>
snip[*****Windows XP SP3******]

I guess this is handled by...

uh? Where's the bug report? This problem was mentioned several times on
the mailing list, so we should have a bug report for it. Can someone
spot an 'eggbasket cannot remove dir' or similar bug?

If it really does not exist yet, we should create a new one.

snip[******* MAC OS X 10.5.6 ******]

This seems to be handled by

  https://bugs.launchpad.net/grok/+bug/180410

right?

I will try to tell in the appropriate bug reports how one can
test/develop a fix for those bugs later today.

>
> By the way Uli,
> A really great patch.

Well, it's yours and Roger's :)

> Now debug.ini and deploy.ini are free of mixed paths.
> Only zope.conf and zdaemon.conf have mixed paths now. We can try to
> look at those once we got this release out of the way (maybe on a new
> bug entry?)

Yes, we can (make a new bugreport for this ;) The importance, however,
could be set to a minor value, as things at least seem to work with
those mixed paths.

So thanks to all contributors here!

Best regards,

Uli

Revision history for this message
Ben Dadsetan (bdadsetan) wrote :

2009/4/17 Uli Fouquet <email address hidden>:
[snip]
>
> uh? Where's the bug report? This problem was mentioned several times on
> the mailing list, so we should have a bug report for it. Can someone
> spot an 'eggbasket cannot remove dir' or similar bug?
>
> If it really does not exist yet, we should create a new one.

I know I know.. I wanted to do it before already but why do today what
you can do next year? :)

Here it is, https://bugs.launchpad.net/grok/+bug/362833

> snip[******* MAC OS X 10.5.6 ******]
>
> This seems to be handled by
>
>  https://bugs.launchpad.net/grok/+bug/180410
>
> right?
>

Correct, that is our other bug.

Thankfully this is a bug limited to Mac and not the other systems.

I actually have found the problem and it is a core python bug. For the
mac platform, when easy_install tries to satisfy dependencies, it
always searches the system path as well even if you are using your
virtualenv --no-site-packages. If a package you need is in the system
path it considers the dependency satisfied and will take no actions.
However as the virtualenv does not take into account the system path
but only the virtual environment's lib\site-packages, it can not find
any package that was already in the system path.

And it all falls apart. I am trying to figure out what the process is
for reporting bugs, supplying patches, discussing bugs and patching
this. I do fear they will not accept to make a 2.5 fix but only 2.6 &
3.0 if we are lucky. So this issue is to stay unless we find a clever
way to get around the problem.

Revision history for this message
Ben Dadsetan (bdadsetan) wrote :

Released with grokproject-1.0a4

Changed in grok:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.