Re: Sketch of not remaking html files (issue 5498093)

[Phil Holmes]

----- Original Message ----- 
From: <address@hidden>
To: <address@hidden>; <address@hidden>
Cc: <address@hidden>; <address@hidden>
Sent: Monday, January 02, 2012 9:41 PM
Subject: Re: Sketch of not remaking html files (issue 5498093)


> a few minor quibbles; I'm on a ferry right now so I can't test a full
> doc build.
>
>
> http://codereview.appspot.com/5498093/diff/1/GNUmakefile.in
> File GNUmakefile.in (right):
>
> http://codereview.appspot.com/5498093/diff/1/GNUmakefile.in#newcode125
> GNUmakefile.in:125: # find $(outdir) -name '*-root' | xargs rm -rf
> please either delete the line entirely, or add a comment explaining why
> you've commented-out that line.  Otherwise I fear that this may become
> another time bomb that may confuse future people working on the build
> process.

I commented it out for my initial trial - easier to put it back if there's a 
problem.  Looks like deleting the line is the best option.

> http://codereview.appspot.com/5498093/diff/1/python/auxiliar/postprocess_html.py
> File python/auxiliar/postprocess_html.py (right):
>
> http://codereview.appspot.com/5498093/diff/1/python/auxiliar/postprocess_html.py#newcode353
> python/auxiliar/postprocess_html.py:353: SourceTime =
> os.path.getmtime(file_name)
> there's an extremely strong convention in python programming to use
> lower-case and underscores, i.e.
>   source_time = os.path.getmtime(file_name)

No problems.  I'm just a Camel caser.

> http://codereview.appspot.com/5498093/diff/1/scripts/build/www_post.py
> File scripts/build/www_post.py (right):
>
> http://codereview.appspot.com/5498093/diff/1/scripts/build/www_post.py#newcode76
> scripts/build/www_post.py:76: sys.exc_clear()
> why do you want to catch this exception?  Surely if mkdir fails, we want
> to display the error message on the command-line?
>
> maybe do something like
>     if not os.path.isdir(out_root):
>         os.mkdir (out_root)
>
> instead?
>
> (I mean, what if the exception that was raised was "you have no disk
> space left on this drive", or something more serious than "that
> directory already exists" ?  -- and yes, I've occasionally tried to
> build lilypond on a drive that wasn't big enough, so this isn't just a
> theoretical concern!)

I spent a while researching online as to how to create a directory if it 
didn't already exist, and consensus was that catching the exception was the 
best option.  There's some sort of issues with race conditions over checking 
whether it exist before creating it - although thinking about this, it's 
possible that this would only occur if more than one process was trying to 
create it at the same time, but anyway....   Probably the very best way is 
to catch the specific exception of directory exists and throw any others. 
Having said that, it's almost certainly theoretical.  If mkdir fails because 
of a disk problem, something else will blow pretty soon.

> http://codereview.appspot.com/5498093/

I'll have a think and put up a new patch.

--
Phil Holmes