Re: [PATCH 3/4] Makefile: Flatten and sort file lists

Pedro Alves
2016-11-15T16:46:29Z

On 11/13/2016 03:46 AM, Simon Marchi wrote:
> I find the big file lists in the Makefile a bit ugly and not very
> practical. Since there are multiple filenames on each line (as much as
> fits in 80 columns), it's not easy to add, remove or change a name in
> the middle. As a result, we have a mix of long and short lines in no
> particular order (ALL_TARGET_OBS is a good example).
>
> I therefore suggest flattening the list (one name per line) and keeping
> it in alphabetical order. The diffs will be much clearer and merge
> conflicts will be easier to resolve.

I agree. I've asked for similar things in other cases for the same
reason.

>
> For the ordering, I aimed at respecting this order:
>
> * foo.c
> * foo-bar.c
> * foobar.c
>
> Which, in non-formal words, could be expressed like so:
>
> * The extension is not taken into account when comparing two filenames
> (except if the filenames are otherwise equal).
> * A filename that is a prefix of another one comes before this other
> one.
> * Underscores and dashes are treated equally (I think we use them both
> for the same purpose).
> * Underscores and dashes come before alphanumeric characters.
>
> Short of any other rule, this could be used as a guideline to determine
> where to add a new filename in the list.

Could/should this be converted into a comment? At least a "keep these
lists sorted" comment, I think. That'll be easier to find than this email
or commit log.

Wouldn't it make sense to sort by files and dirs separately? I.e.,
files first, then dirs, e.g.? You have this, for example:

> + mdebugread.h \
> + memattr.h \
> + memory-map.h \
> + memrange.h \
> + mi/mi-cmds.h \
> + mi/mi-common.h \
> + mi/mi-console.h \
> + mi/mi-getopt.h \
> + mi/mi-main.h \
> + mi/mi-out.h \
> + mi/mi-parse.h \
> + microblaze-tdep.h \
> + mips-linux-tdep.h \
> + mips-tdep.h \
> + mipsnbsd-tdep.h \

I.e., MI files mixed with the other files. I'd expected:

> + mdebugread.h \
> + memattr.h \
> + memory-map.h \
> + memrange.h \
> + microblaze-tdep.h \
> + mips-linux-tdep.h \
> + mips-tdep.h \
> + mipsnbsd-tdep.h \
...
> + mi/mi-cmds.h \
> + mi/mi-common.h \
> + mi/mi-console.h \
> + mi/mi-getopt.h \
> + mi/mi-main.h \
> + mi/mi-out.h \
> + mi/mi-parse.h \

Maybe it's be clearer to move subdir headers to
variables, like we have SUBDIR_PYTHON_SRCS/SUBDIR_PYTHON_OBS,
etc. for sources. But still, for the cases where we don't have
variables.

> SUBDIR_TUI_OBS = \
> + tui.o \
> tui-command.o \
> tui-data.o \
> tui-disasm.o \
> @@ -269,9 +304,9 @@ SUBDIR_TUI_OBS = \
> tui-windata.o \
> tui-wingeneral.o \
> tui-winsource.o \
> - tui.o

Mind the trailing \ left in the last element. Here
and other places.

Thanks,
Pedro Alves