Re: [PATCH v2 5/5] mkfs.xfs: add configuration file parsing support using our own parser

From: Eric Sandeen <sandeen@sandeen.net>
To: "Luis R. Rodriguez" <mcgrof@kernel.org>,
	"Darrick J. Wong" <darrick.wong@oracle.com>
Cc: linux-xfs@vger.kernel.org, jack@suse.com, jeffm@suse.com,
	okurz@suse.com, lpechacek@suse.com, jtulak@redhat.com
Subject: Re: [PATCH v2 5/5] mkfs.xfs: add configuration file parsing support using our own parser
Date: Fri, 18 May 2018 12:09:23 -0500	[thread overview]
Message-ID: <c2e7b213-0dad-7609-7984-e4c897d6bb08@sandeen.net> (raw)
In-Reply-To: <20180518153805.GE24680@garbanzo.do-not-panic.com>

On 5/18/18 10:38 AM, Luis R. Rodriguez wrote:
> On Thu, May 17, 2018 at 08:46:00PM -0700, Darrick J. Wong wrote:
>> On Thu, May 17, 2018 at 10:24:13PM -0500, Eric Sandeen wrote:

...

> Let us recall that the reason for -c set in stone on /@sysconfigdir@/mkfs.xfs.d/
> was to allow clean simple code.
> 
> The MKFS_XFS_CONFIG is simply a comopromise to allow flexibility on a
> full path in case you cannot use the /@sysconfigdir@/mkfs.xfs.d/
> directory.
> 
> Supporting both remains simple.
> 
> If we wanted to support what you suggest, if a user specified -c hoogah
> we'd have to treat multiple possibilities in code, increasing complexity:
> 
>    a) Did the user mean the hoogah in the present directory?
>    b) Did the user mean hoogah in /@sysconfigdir@/mkfs.xfs.d/
> 
> Granted, if the user specified a -c /tmp/hoogah its clearer that the
> full path would be desirable. So, I think what you suggest makes sense
> provided we get rid of a) option and require only an alternative path
> *iff* the first character in the path is '/'. Does this work for all
> cases we wish to support a full path in?

Possibly include "./" as well. i.e.  mkfs.xfs -c ./configdir/myconfig

so:

mkfs.xfs -c hoogah		searches /@sysconfigdir@/mkfs.xfs.d/
mkfs.xfs -c ./hoogah		or
mkfs.xfs -c /path/to/hoogah	do exactly what you'd expect.

>> And the -c
>> option can be specified once to override the environment variable /
>> builtin detaults?

I'd like to drop the environment variable altogether.  If there is a strong
case to be made for keeping it, please make it. :)  (I don't consider the
existence of MKE2FS_CONFIG to be a strong argument, FWIW, because our
semantics & mechanisms are quite different.)

Thanks,
-Eric