Re: [PATCH] mtd-utils: Add configure switches to disable jffsX or ubifs tools

[Thomas Petazzoni <thomas.petazzoni@free-electrons.com>]

Hello,

On Thu, 22 Dec 2016 10:27:36 +0100, David Oberhollenzer wrote:

> That is indeed an interesting consideration. The guides seem to be a bit
> blury on the destinction on enable/disable vs with/without.
> 
> I was under the impression that one should use enable/disable for features
> of a program and with/without for external dependencies or entire programs,
> i.e. "features of a project".

Yes, indeed that the general distinction between enable/disable and
with/without, you're right.

What I found odd in your code is the fact that you're in the end only
accepting yes or no, and the duplication of the AM_CONDITIONAL()
statements.

> > Indeed, using AM_CONDITIONAL() is useful to assign automake variable.
> > Here, BUILD_JFFS and BUILD_UBIFS are apparently only used within the
> > configure.ac, so using AM_CONDITIONAL() is useless.  
> Not quite. They are used in the automake file to skip building mkfs.ubifs
> and the jffsx utils.

Ah, okay. I haven't looked at the code base, just the patch itself, so
I definitely missed that. So an AM_CONDITIONAL() is definitely good.

But regardless of whether you go with AC_ARG_WITH() or AC_ARG_ENABLE(),
I still believe it's a lot more readable to have the AM_CONDITIONAL()
test *after* the AC_ARG_{WITH,ENABLE}.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com