automake-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 2/?] Work around a bug in Solaris make's file-inclusion me


From: Stefano Lattarini
Subject: Re: [PATCH v3 2/?] Work around a bug in Solaris make's file-inclusion mechanism.
Date: Sun, 26 Sep 2010 11:58:09 +0200
User-agent: KMail/1.13.3 (Linux/2.6.30-2-686; KDE/4.4.4; i686; ; )

On Sunday 26 September 2010, Ralf Wildenhues wrote:
> Hello Stefano,
> 
> * Stefano Lattarini wrote on Fri, Sep 24, 2010 at 04:38:29PM CEST:
> > On Wednesday 15 September 2010, Ralf Wildenhues wrote:
> > > I don't think we want subobj11c.test.  What we want instead,
> > > and I agree with your earlier sentiments on this, is unit
> > > testing for the normalize_file_name function.  Which suggests
> > > that it should be in lib/Automake/FileUtils.pm I guess,
> > 
> > I disagree, it would be a text-transformation function which
> > doesn't really belong to that file.
> 
> What I meant was just the functionality of normalizing a file name
> to not contain non-leading multiple slashes.  That would quite
> well fit in FileUtils, and also could be usable for other things
> as well.
Oh.  This is true.
 
> Computing a dependency file name might be done *on top of that*,
Not sure about that, the code in my proposed `compute_depfile_path'
seems too ad-hoc to be easily implemented on the top of another 
function.
> in another function, sure.  But "Misc" is not a good category for
> that, and I don't quite see yet where it would be reusable.
If you mean `compute_depfile_path', well, it wouldn't. But at least,
putting it in a module would make it unit-testable.  That said...
 
> > BTW, in the long run, my idea would be to move *almost all* of
> > automake.in into a "temporary" module `Automake::Main', to ease
> > its unit-testability.
... with this implemented, we could just leave `compute_depfile_path'
and other ad-hoc functions in `Automake::Main', and they could be
easily unit-tested, without the need to introduce a confusing module
`Automake::Misc'.

> > Then we could proceed to refactoring, and split it up in several
> > separate modules where needed, thus improving modularity and
> > clarity.  But this second step should be undertaken only once we
> > have enough testsuite coverage -- no, the present one is
> > definitely not enough IMHO).
> 
> For such an approach, I'd first like to see, and discuss, a design
> layout for the intended structre.
But that would be for the second step; the first step (creation of
`Automake::Main') shouldn't change the structure of automake.in, just
move it to make it easily unit-testable.  Then we could discuss the
design layout for the intended structre, or even decide not to change
the structure at all.
> Not ad-hoc hacking and factorizing like this, that has a good
> chance to create a mess in the long run.

> Which is the reason I'm rejecting this patch for now, sorry.
I'll happily repropose it when the patch queue is stabilized enough
to allow the creation of `Automake::Main'.

> > I'm not trying to do this code reoarganization now, because it
> > would disrupt useful pending patches, at least:
> It could be done in a branch only to be merged strictly afterwards.
Yes, but the merge would imply basically a huge conflict over all 
automake.in, and require a by-hand re-application of those patches
I talked about.  No thanks.

BTW, with this mail are you rejecting also the first part of the
patch ("[PATCH v3] Work around a bug in Solaris make's file-inclusion 
mechanism.")?

Regards,
   Stefano



reply via email to

[Prev in Thread] Current Thread [Next in Thread]