The former is somewhat misnamed. FIND_SOURCE_FILES is *not* a list
of source files---it is a procedure to list source files to its
standard output. FIND_C_SOUCRES sounds as if it is a similar
procedure, which would be implemented much like
FIND_C_SOURCES = $(FIND_SOURCE_FILES) | sed -n -e '/\.c$/p'
but that is not what you did and that is not what you want to have.
Perhaps call it FOUND_C_SOURCES?
I wonder if we can get rid of FIND_SOURCE_FILES that is a mere
procedure and replace its use with a true list of source files.
Would it make the result more pleasant to work with?
Perhaps something like the attached patch, (which would come before
this entire thing as a clean-up, and removing the need for 2/3)?
I dunno.
Using a procedure whose output is fed to xargs has an advantage that
a platform with very short command line limit can still work with
many source files, but the way you create and use COCCI_SOURCES in
this patch would defeat that advantage anyway, so perhaps we can get
away with an approach like this. Having a list of things in $(MAKE)
variable has a longer-term benefit that we could exploit more
parallelism if we wanted to, too.
Makefile | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
The former is somewhat misnamed. FIND_SOURCE_FILES is *not* a list
of source files---it is a procedure to list source files to its
standard output. FIND_C_SOUCRES sounds as if it is a similar
procedure, which would be implemented much like
FIND_C_SOURCES = $(FIND_SOURCE_FILES) | sed -n -e '/\.c$/p'
but that is not what you did and that is not what you want to have.
Perhaps call it FOUND_C_SOURCES?
I wonder if we can get rid of FIND_SOURCE_FILES that is a mere
procedure and replace its use with a true list of source files.
Would it make the result more pleasant to work with?
Perhaps something like the attached patch, (which would come before
this entire thing as a clean-up, and removing the need for 2/3)?
I dunno.
Using a procedure whose output is fed to xargs has an advantage that
a platform with very short command line limit can still work with
many source files, but the way you create and use COCCI_SOURCES in
this patch would defeat that advantage anyway,
COCCI_SOURCES is only used as an input to 'xargs', so that advantage
is not defeated.
quoted hunk
so perhaps we can get
away with an approach like this. Having a list of things in $(MAKE)
variable has a longer-term benefit that we could exploit more
parallelism if we wanted to, too.
Makefile | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
The former is somewhat misnamed. FIND_SOURCE_FILES is *not* a list
of source files---it is a procedure to list source files to its
standard output. FIND_C_SOUCRES sounds as if it is a similar
procedure, which would be implemented much like
FIND_C_SOURCES = $(FIND_SOURCE_FILES) | sed -n -e '/\.c$/p'
but that is not what you did and that is not what you want to have.
Perhaps call it FOUND_C_SOURCES?
I wonder if we can get rid of FIND_SOURCE_FILES that is a mere
procedure and replace its use with a true list of source files.
Would it make the result more pleasant to work with?
Perhaps something like the attached patch, (which would come before
this entire thing as a clean-up, and removing the need for 2/3)?
I dunno.
Using a procedure whose output is fed to xargs has an advantage that
a platform with very short command line limit can still work with
many source files, but the way you create and use COCCI_SOURCES in
this patch would defeat that advantage anyway,
COCCI_SOURCES is only used as an input to 'xargs', so that advantage
is not defeated.
I think it still does matter; the relevant snippet is as follows:
if ! echo $(COCCI_SOURCES) | xargs $$limit \
$(SPATCH) --sp-file $< $(SPATCH_FLAGS) \
>$@+ 2>$@.log; \
which means that a really big COCCI_SOURCES could exceed the limit.
That being said, COCCI_SOURCES should be smaller than the future
SOURCE_FILES variable since we're only taking %.c files (and filtering
out some of them too!).
I dunno, either. I'm mostly in favour of this change since it makes a
lot of sense to keep lists in make variables if possible as opposed to
command invocations. I guess worst case, if someone complains in the
future, we can always change it back.
quoted
so perhaps we can get
away with an approach like this. Having a list of things in $(MAKE)
variable has a longer-term benefit that we could exploit more
parallelism if we wanted to, too.
Makefile | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
The former is somewhat misnamed. FIND_SOURCE_FILES is *not* a list
of source files---it is a procedure to list source files to its
standard output. FIND_C_SOUCRES sounds as if it is a similar
procedure, which would be implemented much like
FIND_C_SOURCES = $(FIND_SOURCE_FILES) | sed -n -e '/\.c$/p'
but that is not what you did and that is not what you want to have.
Perhaps call it FOUND_C_SOURCES?
I wonder if we can get rid of FIND_SOURCE_FILES that is a mere
procedure and replace its use with a true list of source files.
Would it make the result more pleasant to work with?
Perhaps something like the attached patch, (which would come before
this entire thing as a clean-up, and removing the need for 2/3)?
I dunno.
Using a procedure whose output is fed to xargs has an advantage that
a platform with very short command line limit can still work with
many source files, but the way you create and use COCCI_SOURCES in
this patch would defeat that advantage anyway,
COCCI_SOURCES is only used as an input to 'xargs', so that advantage
is not defeated.
I think it still does matter; the relevant snippet is as follows:
if ! echo $(COCCI_SOURCES) | xargs $$limit \
$(SPATCH) --sp-file $< $(SPATCH_FLAGS) \
>$@+ 2>$@.log; \
which means that a really big COCCI_SOURCES could exceed the limit.
Oh, you're both right.
That being said, COCCI_SOURCES should be smaller than the future
SOURCE_FILES variable since we're only taking %.c files (and filtering
out some of them too!).
We could also argue that Coccinelle only runs on platforms that have a
reasonably large command line arg limit, and the number of our source
files is way below that, so it won't matter in the foreseeable future.
(Furthermore, 'echo' is often a shell builtin command, and I don't
think that the platform's argument size limit applies to them. At
least the 'echo' of dash, Bash, ksh, ksh93, mksh, and BusyBox sh can
deal with at least 10 million arguments; the platform limit is
somewhere around 147k)
Here, however, the list of source files is passed as argument to
non-builtin commands, that also might be used on
cmdline-arg-limit-challenged platforms.
The former is somewhat misnamed. FIND_SOURCE_FILES is *not* a list
of source files---it is a procedure to list source files to its
standard output. FIND_C_SOUCRES sounds as if it is a similar
procedure, which would be implemented much like
FIND_C_SOURCES = $(FIND_SOURCE_FILES) | sed -n -e '/\.c$/p'
but that is not what you did and that is not what you want to have.
Perhaps call it FOUND_C_SOURCES?
I wonder if we can get rid of FIND_SOURCE_FILES that is a mere
procedure and replace its use with a true list of source files.
Would it make the result more pleasant to work with?
Perhaps something like the attached patch, (which would come before
this entire thing as a clean-up, and removing the need for 2/3)?
I dunno.
Using a procedure whose output is fed to xargs has an advantage that
a platform with very short command line limit can still work with
many source files, but the way you create and use COCCI_SOURCES in
this patch would defeat that advantage anyway,
COCCI_SOURCES is only used as an input to 'xargs', so that advantage
is not defeated.
I think it still does matter; the relevant snippet is as follows:
if ! echo $(COCCI_SOURCES) | xargs $$limit \
$(SPATCH) --sp-file $< $(SPATCH_FLAGS) \
>$@+ 2>$@.log; \
which means that a really big COCCI_SOURCES could exceed the limit.
Oh, you're both right.
quoted
That being said, COCCI_SOURCES should be smaller than the future
SOURCE_FILES variable since we're only taking %.c files (and filtering
out some of them too!).
We could also argue that Coccinelle only runs on platforms that have a
reasonably large command line arg limit, and the number of our source
files is way below that, so it won't matter in the foreseeable future.
Good point.
(Furthermore, 'echo' is often a shell builtin command, and I don't
think that the platform's argument size limit applies to them. At
least the 'echo' of dash, Bash, ksh, ksh93, mksh, and BusyBox sh can
deal with at least 10 million arguments; the platform limit is
somewhere around 147k)
Here, however, the list of source files is passed as argument to
non-builtin commands, that also might be used on
cmdline-arg-limit-challenged platforms.
After doing a bit of research, I think that I agree with you. It seems
like the max command-line length for CMD on Windows is 8191 characters.
However, after running the following,
$ git ls-files '*.[hcS]' '*.sh' ':!*[tp][0-9][0-9][0-9][0-9]*' ':!contrib' | wc -c
12779
we can see that the command-line length would definitely exceed the max
length so xargs would be required. As a result, we should probably just
keep the existing xargs invocations.