Re: User extensions
[off-list] Reviving an oldish thread... Reference: http://lists.gnu.org/archive/html/automake-patches/2010-11/msg2.html On Monday 01 November 2010, Ralf Wildenhues wrote: * Valentin David wrote on Mon, Nov 01, 2010 at 09:41:47PM CET: I hope I will be able to prepare the new patches pretty soon. Cool. Whatever you send, several small changes (each being a logical entity) is always easier to review than one large one. Hello Valentin. Have you had time to respin the patches yet? I find the feature you're implementing very useful and promising, and thus I'd like to know if there are progresses, and if I could help in any way. Thanks, Stefano
Re: User extensions
Hello Pippijn. On Saturday 13 November 2010, Pippijn van wrote: On Mon, Nov 01, 2010 at 08:18:44PM +0100, Ralf Wildenhues wrote: Ideally, I would like to see testsuite coverage for each code path (branch coverage) for new code. I understand that only Stefano is able to produce this in reasonable amount of time, so whatever you guys can manage is better than nothing. Hi, not having followed the code discussion very closely (just hoping it will be in git, soon, so I can start using it, as I have been waiting for something like this for a long time), I just wonder what you mean by branch coverage. If this is execution graph node coverage, then I agree, but if you mean edge coverage, I don't know how you want to achieve this. Any O(x) where x1 algorithm anywhere in the code makes it impossible. If I'm wrong, I'd like to know how Stefano produces it. Well, basically by hand. I think that Ralf was just saying that, since I'm familiar with the automake testsuite and interested in extending it, since I presently have definitely more time to do so than Ralf has, and, above all, since there's no other regular contributor to automake ATM, if someone is able to add proper testing for the new feature, that's either me or the original contributor of the feature (Valentin David). But it's not like I have a silver bullet to write proper tests which attain complete branch coverage; I just proceed by common sense, trying to maximize such coverage. That's all, sadly. Regards, Stefano
Re: User extensions
* Stefano Lattarini wrote on Sun, Nov 14, 2010 at 01:36:16PM CET: But it's not like I have a silver bullet to write proper tests which attain complete branch coverage; I just proceed by common sense, trying to maximize such coverage. That's all, sadly. A while ago I posted instructions on how to generate coverage information for the perl parts of Automake, including results: http://thread.gmane.org/gmane.comp.sysutils.automake.general/11088 That doesn't help for shell or makefile snippets, but it's a whole lot better than nothing. Cheers, Ralf
Re: User extensions
On Sunday 14 November 2010, Ralf Wildenhues wrote: * Stefano Lattarini wrote on Sun, Nov 14, 2010 at 01:36:16PM CET: But it's not like I have a silver bullet to write proper tests which attain complete branch coverage; I just proceed by common sense, trying to maximize such coverage. That's all, sadly. A while ago I posted instructions on how to generate coverage information for the perl parts of Automake, including results: http://thread.gmane.org/gmane.comp.sysutils.automake.general/11088 Interesting. Somehow I missed (or I forgot) that. Thanks for the reminder. That doesn't help for shell or makefile snippets, but it's a whole lot better than nothing. Cheers, Ralf Regards, Stefano
Re: User extensions
Hello Pippijn. On Saturday 13 November 2010, Pippijn van wrote: On Mon, Nov 01, 2010 at 08:18:44PM +0100, Ralf Wildenhues wrote: Ideally, I would like to see testsuite coverage for each code path (branch coverage) for new code. I understand that only Stefano is able to produce this in reasonable amount of time, so whatever you guys can manage is better than nothing. Hi, not having followed the code discussion very closely (just hoping it will be in git, soon, so I can start using it, as I have been waiting for something like this for a long time), I just wonder what you mean by branch coverage. If this is execution graph node coverage, then I agree, but if you mean edge coverage, I don't know how you want to achieve this. Any O(x) where x1 algorithm anywhere in the code makes it impossible. If I'm wrong, I'd like to know how Stefano produces it. Well, basically by hand. I think that Ralf was just saying that, since I'm familiar with the automake testsuite and interested in extending it, since I presently have definitely more time to do so than Ralf has, and, above all, since there's no other regular contributor to automake ATM, if someone is able to add proper testing for the new feature, that's either me or the original contributor of the feature (Valentin David). But it's not like I have a silver bullet to write proper tests which attain complete branch coverage; I just proceed by common sense, trying to maximize such coverage. That's all, sadly. Regards, Stefano
Re: User extensions
* Stefano Lattarini wrote on Sun, Nov 14, 2010 at 01:36:16PM CET: But it's not like I have a silver bullet to write proper tests which attain complete branch coverage; I just proceed by common sense, trying to maximize such coverage. That's all, sadly. A while ago I posted instructions on how to generate coverage information for the perl parts of Automake, including results: http://thread.gmane.org/gmane.comp.sysutils.automake.general/11088 That doesn't help for shell or makefile snippets, but it's a whole lot better than nothing. Cheers, Ralf
Re: User extensions
On Sunday 14 November 2010, Ralf Wildenhues wrote: * Stefano Lattarini wrote on Sun, Nov 14, 2010 at 01:36:16PM CET: But it's not like I have a silver bullet to write proper tests which attain complete branch coverage; I just proceed by common sense, trying to maximize such coverage. That's all, sadly. A while ago I posted instructions on how to generate coverage information for the perl parts of Automake, including results: http://thread.gmane.org/gmane.comp.sysutils.automake.general/11088 Interesting. Somehow I missed (or I forgot) that. Thanks for the reminder. That doesn't help for shell or makefile snippets, but it's a whole lot better than nothing. Cheers, Ralf Regards, Stefano
Re: User extensions
On Mon, Nov 01, 2010 at 08:18:44PM +0100, Ralf Wildenhues wrote: Ideally, I would like to see testsuite coverage for each code path (branch coverage) for new code. I understand that only Stefano is able to produce this in reasonable amount of time, so whatever you guys can manage is better than nothing. Hi, not having followed the code discussion very closely (just hoping it will be in git, soon, so I can start using it, as I have been waiting for something like this for a long time), I just wonder what you mean by branch coverage. If this is execution graph node coverage, then I agree, but if you mean edge coverage, I don't know how you want to achieve this. Any O(x) where x1 algorithm anywhere in the code makes it impossible. If I'm wrong, I'd like to know how Stefano produces it. -- Pippijn van Steenhoven signature.asc Description: Digital signature
Re: User extensions
On Mon, Nov 01, 2010 at 08:18:44PM +0100, Ralf Wildenhues wrote: Ideally, I would like to see testsuite coverage for each code path (branch coverage) for new code. I understand that only Stefano is able to produce this in reasonable amount of time, so whatever you guys can manage is better than nothing. Hi, not having followed the code discussion very closely (just hoping it will be in git, soon, so I can start using it, as I have been waiting for something like this for a long time), I just wonder what you mean by branch coverage. If this is execution graph node coverage, then I agree, but if you mean edge coverage, I don't know how you want to achieve this. Any O(x) where x1 algorithm anywhere in the code makes it impossible. If I'm wrong, I'd like to know how Stefano produces it. -- Pippijn van Steenhoven signature.asc Description: Digital signature
Re: User extensions
Hi Valentin,
let's move to the -patches list. And sorry for the long delay.
* Valentin David wrote on Fri, Sep 03, 2010 at 06:56:53PM CEST:
I propose a patch as attached.
* The lang_*_rewrite are added to the Language structure. The default
is lang_sub_obj. They do not return anymore the object extension
because the field 'output_extensions' already computes it.
As Stefano already suggested, this change is fine for master in a patch
by itself, and when it passes 'make check', regardless of the rest.
Please format patches in the way that is done throughout Automake
history (see HACKING), with ChangeLog entries, test suite additions,
and commit including modifications induced by ./bootstrap (given the
most recent stable Autoconf version in $PATH). Thanks. Generated files
need not be listed in the log.
* Compilers that generate source files might still generate dependency files.
* --libdir= can be called several times, the arguments can also have a
list of paths separated by a colon. Empty paths correspond to the
original libdir.
Due to the colon/semicolon issues, how about we just ignore the issue
for now and require one --libdir argument per directory?
Also, the version issues can end up fairly intricate: we cannot really
expect automake 1.13 to successfully load the same Perl modules that
were meant for 1.12; there are *lots* of things that can go wrong, and
we just opened a can of API worms. Still, I would really like to see
progress here, so let's for the moment not yet settle on a system-wide
(set of) directory(ies) where such additional third-party files should
end up at. Another issue is overriding Automake's own .am files; we've
always treated them as requiring to be in sync with the automake.in
version and it could end up being pretty tricky to specify their API as
public in any way.
Also, I like the approach of calling the whole feature experimental and
at the same time asking users to tell us which features from automake.in
they need so we can document them and add testsuite coverage for them so
we can be sure to not break them in the future.
* For all libdir, all libdir/lang/*.pm are loaded. This happens right
after parsing the options.
The test tests/user_def_lang.test should show how the feature works.
I can understand that loading perl files is not really nice because
there easily can be problems of backward compatibility.
Exactly.
Ideally, I would like to see testsuite coverage for each code path
(branch coverage) for new code. I understand that only Stefano is
able to produce this in reasonable amount of time, so whatever you guys
can manage is better than nothing. The new code should have a NEWS
entry and at least some documentation in the manual. You can target
a new branch that is meant for eventual inclusion into master, let's
name it user-ext, and whether you base patches off of maint or of master
I don't care. The latter might be easier for you.
Some nits below, in addition (or in contrast ;-) to Stefano's.
--- a/automake.in
+++ b/automake.in
@@ -1858,17 +1871,15 @@ sub handle_single_transform ($%)
}
}
- # Note: computed subr call. The language rewrite function
- # should return one of the LANG_* constants. It could
- # also return a list whose first value is such a constant
- # and whose second value is a new source extension which
- # should be applied. This means this particular language
- # generates another source file which we must then process
- # further.
- my $subr = \{'lang_' . $lang-name . '_rewrite'};
- my ($r, $source_extension)
- = $subr ($directory, $base, $extension,
+ # The language rewrite function should return one of the
+# LANG_* constants.
+ my $r
+ = {$lang-rewrite} ($directory, $base, $extension,
$nonansi_obj, $have_per_exec_flags, $var);
+
+ my $output_extension
+ = ({$lang-output_extensions} ($extension))[0];
+
# Skip this entry if we were asked not to process it.
next if $r == LANG_IGNORE;
@@ -1876,9 +1887,9 @@ sub handle_single_transform ($%)
$linker = $lang-linker;
my $this_obj_ext;
- if (defined $source_extension)
+ if ($output_extension ne '.$(OBJEXT)')
Why this? This looks wrong, but I cannot judge because I don't
understand why your patch would need this.
{
- $this_obj_ext = $source_extension;
+ $this_obj_ext = $output_extension;
$derived_source = 1;
}
elsif ($lang-ansi)
@@ -2056,10 +2067,19 @@ sub handle_single_transform ($%)
$lang-target_hook ($aggregate, $object, $full, %transform);
}
+ # Transform .o or $o file into .P file (for automatic
+ # dependency code).
+ if ($lang $lang-autodep ne
Re: User extensions
On Mon, Nov 1, 2010 at 8:18 PM, Ralf Wildenhues ralf.wildenh...@gmx.de wrote:
* Valentin David wrote on Fri, Sep 03, 2010 at 06:56:53PM CEST:
* --libdir= can be called several times, the arguments can also have a
list of paths separated by a colon. Empty paths correspond to the
original libdir.
Due to the colon/semicolon issues, how about we just ignore the issue
for now and require one --libdir argument per directory?
That is OK for me. This one is more important as it is used in the test suite.
Also, I like the approach of calling the whole feature experimental and
at the same time asking users to tell us which features from automake.in
they need so we can document them and add testsuite coverage for them so
we can be sure to not break them in the future.
The problem is that the important variables in automake.in are all
local (my). So even if you set the module to package Automake, you
still cannot access to them without modifying automake.in. We should
have accessors to most of them. For example, in the patch I sent,
there is a bug in the test case, as it tries to add something
%clean_files but actually does not.
@@ -1876,9 +1887,9 @@ sub handle_single_transform ($%)
$linker = $lang-linker;
my $this_obj_ext;
- if (defined $source_extension)
+ if ($output_extension ne '.$(OBJEXT)')
Why this? This looks wrong, but I cannot judge because I don't
understand why your patch would need this.
lang_..._rewrite sometimes returned a pair where the rhs to be
$source_extension, which correspond to the extension of the generated
file. However, there is already another entry that does this. When I
rewrote the lang_..._rewrite, I just dropped this variable. Now it
uses $output_extension which is always defined ($source_extension was
also not really a wise name given it is the generated file, not really
a source). Then to check if it is derived source (which means needs
another compiler after), the test is to check the extension needed of
the output.
{
- $this_obj_ext = $source_extension;
+ $this_obj_ext = $output_extension;
$derived_source = 1;
}
elsif ($lang-ansi)
@@ -2056,10 +2067,19 @@ sub handle_single_transform ($%)
$lang-target_hook ($aggregate, $object, $full, %transform);
}
+ # Transform .o or $o file into .P file (for automatic
+ # dependency code).
+ if ($lang $lang-autodep ne 'no')
+ {
+ my $depfile = $object;
+ $depfile =~ s/\.([^.]*)$/.P$1/;
+ $depfile =~ s/\$\(OBJEXT\)$/o/;
+ $dep_files{dirname ($depfile) . '/$(DEPDIR)/'
+ . basename ($depfile)} = 1;
+ }
+
if ($derived_source)
{
- prog_error ($lang-name . has automatic dependency tracking)
- if $lang-autodep ne 'no';
This removal (without replacement) strikes me as weird. What did you
need it for? We should have a testsuite addition that exposes the
error, and see that a fix of yours keeps the old failure working.
It is wrong to assume that a compiler that generates source files
cannot have automatic dependencies. Take Stratego, this compiler takes
.str files and generate .c files. But several .str files can be
dependency to one .c file. And the compiler actually generate a
dependency file. The current way is now to have a -include $(wildcard
*.dep) in each file. And this is not really good thing.
See http://strategoxt.org/
Secondly this error message can appear only on a bug in Automake, and
cannot be triggered from the files of the user. So you cannot really
make a test that fails here. And now that the autodep section (moved
right before) can handle derived source, this assertion is not useful
anymore.
+END
+
+mkdir -p lib/lang
+cat lib/lang/foo.pm 'END'
+register_language (# Short name of the language (c, f77...).
+ 'name' = foo,
+ # Nice name of the language (C, Fortran 77...).
+ 'Name' = Foo,
+ # List of configure variables which must be defined.
+ 'config_vars' = ['FOO'],
+ 'autodep' = 'FOO',
+
+ # Name of the compiling variable (COMPILE).
+ 'compiler' = FOOCOMPILE,
+ # Content of the compiling variable.
+ 'compile' = '$(FOO) $(FOOFLAGS)',
+ # Flag to require compilation without linking (-c).
+ 'extensions' = ['.foo'],
+ # A subroutine to compute a list of possible extensions of
+ # the product given the input extensions.
+ # (defaults to a subroutine which returns ('.$(OBJEXT)',
'.lo'))
+ 'output_extensions' = sub { return ('.c',); },
+ # A list of flag variables used in 'compile'.
+ # (defaults to [])
+ 'flags' = [FOOFLAGS],
+
+
Re: User extensions
* Valentin David wrote on Mon, Nov 01, 2010 at 09:41:47PM CET:
On Mon, Nov 1, 2010 at 8:18 PM, Ralf Wildenhues wrote:
Also, I like the approach of calling the whole feature experimental and
at the same time asking users to tell us which features from automake.in
they need so we can document them and add testsuite coverage for them so
we can be sure to not break them in the future.
The problem is that the important variables in automake.in are all
local (my). So even if you set the module to package Automake, you
still cannot access to them without modifying automake.in. We should
have accessors to most of them. For example, in the patch I sent,
there is a bug in the test case, as it tries to add something
%clean_files but actually does not.
Good points. Accessors sound like a good idea; more generally, it will
probably end up being cleanest to do some restructuring so that a more
stable API can be provided.
The testsuite additions should ensure that such things as cleaning files
actually work.
@@ -1876,9 +1887,9 @@ sub handle_single_transform ($%)
$linker = $lang-linker;
my $this_obj_ext;
- if (defined $source_extension)
+ if ($output_extension ne '.$(OBJEXT)')
Why this? This looks wrong, but I cannot judge because I don't
understand why your patch would need this.
lang_..._rewrite sometimes returned a pair where the rhs to be
$source_extension, which correspond to the extension of the generated
file. However, there is already another entry that does this. When I
rewrote the lang_..._rewrite, I just dropped this variable. Now it
uses $output_extension which is always defined ($source_extension was
also not really a wise name given it is the generated file, not really
a source). Then to check if it is derived source (which means needs
another compiler after), the test is to check the extension needed of
the output.
OK.
{
- $this_obj_ext = $source_extension;
+ $this_obj_ext = $output_extension;
$derived_source = 1;
}
elsif ($lang-ansi)
@@ -2056,10 +2067,19 @@ sub handle_single_transform ($%)
$lang-target_hook ($aggregate, $object, $full, %transform);
}
+ # Transform .o or $o file into .P file (for automatic
+ # dependency code).
+ if ($lang $lang-autodep ne 'no')
+ {
+ my $depfile = $object;
+ $depfile =~ s/\.([^.]*)$/.P$1/;
+ $depfile =~ s/\$\(OBJEXT\)$/o/;
+ $dep_files{dirname ($depfile) . '/$(DEPDIR)/'
+ . basename ($depfile)} = 1;
+ }
+
if ($derived_source)
{
- prog_error ($lang-name . has automatic dependency tracking)
- if $lang-autodep ne 'no';
This removal (without replacement) strikes me as weird. What did you
need it for? We should have a testsuite addition that exposes the
error, and see that a fix of yours keeps the old failure working.
It is wrong to assume that a compiler that generates source files
cannot have automatic dependencies.
Sure. What I'm not sure is whether the rest of the Automake code
doesn't provide the required functionality only for such languages.
Hmm, this was introduced in Release-1-4f-43-g081f2d5, which is really
long ago; it might still be valid but then again maybe not.
How about we remove it at the point where we add testsuite coverage for
a language that has this feature (so we can be fairly sure that Automake
is prepared)?
Take Stratego, this compiler takes
.str files and generate .c files. But several .str files can be
dependency to one .c file. And the compiler actually generate a
dependency file. The current way is now to have a -include $(wildcard
*.dep) in each file. And this is not really good thing.
See http://strategoxt.org/
Thanks for the explanations.
Secondly this error message can appear only on a bug in Automake, and
cannot be triggered from the files of the user. So you cannot really
make a test that fails here. And now that the autodep section (moved
right before) can handle derived source, this assertion is not useful
anymore.
+register_language (# Short name of the language (c, f77...).
+ 'name' = foo,
+ # Nice name of the language (C, Fortran 77...).
+ 'Name' = Foo,
+ # List of configure variables which must be defined.
+ 'config_vars' = ['FOO'],
+ 'autodep' = 'FOO',
+
+ # Name of the compiling variable (COMPILE).
+ 'compiler' = FOOCOMPILE,
+ # Content of the compiling variable.
+ 'compile' = '$(FOO) $(FOOFLAGS)',
+ # Flag to require compilation without linking (-c).
+ 'extensions' = ['.foo'],
+ # A subroutine to compute a list of possible extensions of
+
Re: User extensions
On Wed, Oct 27, 2010 at 02:13:00PM +0200, Valentin David wrote: On Sat, Oct 23, 2010 at 5:27 PM, Ralf Wildenhues ralf.wildenh...@gmx.de wrote: I haven't looked at the patch in detail yet, but will, now that the assignment papers are done (thanks!). It is done. Does this mean the feature will soon be in git? -- Pippijn van Steenhoven signature.asc Description: Digital signature
Re: User extensions
Hi Valentin,
let's move to the -patches list. And sorry for the long delay.
* Valentin David wrote on Fri, Sep 03, 2010 at 06:56:53PM CEST:
I propose a patch as attached.
* The lang_*_rewrite are added to the Language structure. The default
is lang_sub_obj. They do not return anymore the object extension
because the field 'output_extensions' already computes it.
As Stefano already suggested, this change is fine for master in a patch
by itself, and when it passes 'make check', regardless of the rest.
Please format patches in the way that is done throughout Automake
history (see HACKING), with ChangeLog entries, test suite additions,
and commit including modifications induced by ./bootstrap (given the
most recent stable Autoconf version in $PATH). Thanks. Generated files
need not be listed in the log.
* Compilers that generate source files might still generate dependency files.
* --libdir= can be called several times, the arguments can also have a
list of paths separated by a colon. Empty paths correspond to the
original libdir.
Due to the colon/semicolon issues, how about we just ignore the issue
for now and require one --libdir argument per directory?
Also, the version issues can end up fairly intricate: we cannot really
expect automake 1.13 to successfully load the same Perl modules that
were meant for 1.12; there are *lots* of things that can go wrong, and
we just opened a can of API worms. Still, I would really like to see
progress here, so let's for the moment not yet settle on a system-wide
(set of) directory(ies) where such additional third-party files should
end up at. Another issue is overriding Automake's own .am files; we've
always treated them as requiring to be in sync with the automake.in
version and it could end up being pretty tricky to specify their API as
public in any way.
Also, I like the approach of calling the whole feature experimental and
at the same time asking users to tell us which features from automake.in
they need so we can document them and add testsuite coverage for them so
we can be sure to not break them in the future.
* For all libdir, all libdir/lang/*.pm are loaded. This happens right
after parsing the options.
The test tests/user_def_lang.test should show how the feature works.
I can understand that loading perl files is not really nice because
there easily can be problems of backward compatibility.
Exactly.
Ideally, I would like to see testsuite coverage for each code path
(branch coverage) for new code. I understand that only Stefano is
able to produce this in reasonable amount of time, so whatever you guys
can manage is better than nothing. The new code should have a NEWS
entry and at least some documentation in the manual. You can target
a new branch that is meant for eventual inclusion into master, let's
name it user-ext, and whether you base patches off of maint or of master
I don't care. The latter might be easier for you.
Some nits below, in addition (or in contrast ;-) to Stefano's.
--- a/automake.in
+++ b/automake.in
@@ -1858,17 +1871,15 @@ sub handle_single_transform ($%)
}
}
- # Note: computed subr call. The language rewrite function
- # should return one of the LANG_* constants. It could
- # also return a list whose first value is such a constant
- # and whose second value is a new source extension which
- # should be applied. This means this particular language
- # generates another source file which we must then process
- # further.
- my $subr = \{'lang_' . $lang-name . '_rewrite'};
- my ($r, $source_extension)
- = $subr ($directory, $base, $extension,
+ # The language rewrite function should return one of the
+# LANG_* constants.
+ my $r
+ = {$lang-rewrite} ($directory, $base, $extension,
$nonansi_obj, $have_per_exec_flags, $var);
+
+ my $output_extension
+ = ({$lang-output_extensions} ($extension))[0];
+
# Skip this entry if we were asked not to process it.
next if $r == LANG_IGNORE;
@@ -1876,9 +1887,9 @@ sub handle_single_transform ($%)
$linker = $lang-linker;
my $this_obj_ext;
- if (defined $source_extension)
+ if ($output_extension ne '.$(OBJEXT)')
Why this? This looks wrong, but I cannot judge because I don't
understand why your patch would need this.
{
- $this_obj_ext = $source_extension;
+ $this_obj_ext = $output_extension;
$derived_source = 1;
}
elsif ($lang-ansi)
@@ -2056,10 +2067,19 @@ sub handle_single_transform ($%)
$lang-target_hook ($aggregate, $object, $full, %transform);
}
+ # Transform .o or $o file into .P file (for automatic
+ # dependency code).
+ if ($lang $lang-autodep ne
Re: User extensions
* Pippijn van Steenhoven wrote on Mon, Nov 01, 2010 at 12:43:57PM CET: On Wed, Oct 27, 2010 at 02:13:00PM +0200, Valentin David wrote: On Sat, Oct 23, 2010 at 5:27 PM, Ralf Wildenhues wrote: I haven't looked at the patch in detail yet, but will, now that the assignment papers are done (thanks!). It is done. Does this mean the feature will soon be in git? Let's see; it would be nice indeed. Ralf
Re: User extensions
On Wednesday 27 October 2010, Valentin David wrote:
On Sat, Oct 23, 2010 at 4:05 PM, Stefano Lattarini
stefano.lattar...@gmail.com wrote:
Hi Valentin. Thank you very much for your patch implementing this
much-desired automake feature. Here is a preliminary review...
On Friday 03 September 2010, Valentin David wrote:
* The lang_*_rewrite are added to the Language structure. The default
is lang_sub_obj. They do not return anymore the object extension
because the field 'output_extensions' already computes it.
Hmm... looking at the git diffs, this seems like a useful refactoring
by itself. Could this be done in separate, preparatory patch preceding
the implementation of the user extensions support proper?
Sure, I can make a series of patches like that. But there might be
also a lot of refactoring afterwards as well.
Of course; I was just pointing out that the preliminary refactoring your
patch requires is IMHO useful also by itself (since it makes the code
simpler and somewhat better incapsulated), and not just in sight of the
implementation of your User extensions feature. That's why I'd like to
see it done in a separate patch. Not a big deal, though.
For example language
dependent code should probably all move in a different file and be
imported via the extension thing.
I heartily agree.
* Compilers that generate source files might still generate dependency
files.
* --libdir= can be called several times, the arguments can also have a
list of paths separated by a colon.
We should really use the system PATH separator here, for better portability
(such PATH separator is already computed and AC_SUBST'd by configure as
`...@path_separator@').
If you say so. Is it normal to have different syntax depending from
which shell you call automake?
It's not dependent on the shell, but on the system. This is an unfortunate
necessity due (I think) to the fact that something like `C:\foo\bar' is a
perfectly valid *and natural* absolute path on windows, so that `:' cannot
be portably used to separate different entries in a list of file or
direrctory paths.
Empty paths correspond to the original libdir.
I guess this is done so that the user can override the previously-passed
custom libdirs with the default one, without having to know its complete
path -- right? I think this is useful and smart, but couldn't we find a
better speacial value than empty to indicate the default libdir?
I thought I have seen it somewhere else, but I do not remember which
software.
If you say there are other significant software packages doing something
similar to your proposed implementation, then I'd like to stick to it, as
I prefer to be consistent with pre-existing policies whenever possible.
Also, in our case, being consistent with the `PATH' environment variable
(an empty entry means the current directory) is not a priority IMHO (and
I personally find that `PATH' policy suboptimal to say the least).
If you have any suggestion instead of empty, then shoot.
In fact I don't. So let's just keep it as it is (my objection was
bike-shedding anyway, and not worth losing other time upon).
The test tests/user_def_lang.test should show how the feature works.
We'll need much deeper test than the one provided by your patch (I can help
writing them if you want).
I do agree. I did more modifications since that patch, and I
understood it was not good.
I'm glad you agree on this.
I can understand that loading perl files is not really nice because
there easily can be problems of backward compatibility.
But it's surely better than the present situation IMHO.
It also mean that an API documentation will be required in the future.
For the moment we could just declare your feauture as experimental and
subject to changes, until things settle down. Anyway, using an
experimental but *documented and supported* feature is IMO way better
then having to maintain a patch against automake.
+ my $r
+ = {$lang-rewrite} ($directory, $base, $extension,
$nonansi_obj, $have_per_exec_flags, $var);
What about a saner indentation here? As in:
my $r = {$lang-rewrite} ($directory, $base, $extension,
$nonansi_obj, $have_per_exec_flags,
$var);
[...]
The function implementation below doesn't respect the GCS (GNU coding
standards) w.r.t. use of white spaces, indentation and brackets placement.
And while I dislike what the GCS mandate on these matters, we should
strive to be consistent anyway.
I trusted emacs...
I don't use emacs ;-)
+sub libfile ($)
+{
+ my ($f) = @_;
+ foreach (@userlibdir) {
+return $_/$f
+ if -r $_/$f;
Wouldn't a `-f $_/$f' be more correct here?
I mean, `-r FILE' succeeds even if FILE is indeed a (readable) directory,
but we are looking for regular files.
Is there a case we want to know if a file
Re: User extensions
On Sat, Oct 23, 2010 at 4:05 PM, Stefano Lattarini
stefano.lattar...@gmail.com wrote:
Hi Valentin. Thank you very much for your patch implementing this
much-desired automake feature. Here is a preliminary review...
On Friday 03 September 2010, Valentin David wrote:
* The lang_*_rewrite are added to the Language structure. The default
is lang_sub_obj. They do not return anymore the object extension
because the field 'output_extensions' already computes it.
Hmm... looking at the git diffs, this seems like a useful refactoring
by itself. Could this be done in separate, preparatory patch preceding
the implementation of the user extensions support proper?
Sure, I can make a series of patches like that. But there might be
also a lot of refactoring afterwards as well. For example language
dependent code should probably all move in a different file and be
imported via the extension thing.
* Compilers that generate source files might still generate dependency files.
* --libdir= can be called several times, the arguments can also have a
list of paths separated by a colon.
We should really use the system PATH separator here, for better portability
(such PATH separator is already computed and AC_SUBST'd by configure as
`...@path_separator@').
If you say so. Is it normal to have different syntax depending from
which shell you call automake?
Empty paths correspond to the original libdir.
I guess this is done so that the user can override the previously-passed
custom libdirs with the default one, without having to know its complete
path -- right? I think this is useful and smart, but couldn't we find a
better speacial value than empty to indicate the default libdir?
I thought I have seen it somewhere else, but I do not remember which
software. If you have any suggestion instead of empty, then shoot.
The test tests/user_def_lang.test should show how the feature works.
We'll need much deeper test than the one provided by your patch (I can help
writing them if you want).
I do agree. I did more modifications since that patch, and I
understood it was not good.
I can understand that loading perl files is not really nice because
there easily can be problems of backward compatibility.
But it's surely better than the present situation IMHO.
It also mean that an API documentation will be required in the future.
+ my $r
+ = {$lang-rewrite} ($directory, $base, $extension,
$nonansi_obj, $have_per_exec_flags, $var);
What about a saner indentation here? As in:
my $r = {$lang-rewrite} ($directory, $base, $extension,
$nonansi_obj, $have_per_exec_flags,
$var);
[...]
The function implementation below doesn't respect the GCS (GNU coding
standards) w.r.t. use of white spaces, indentation and brackets placement.
And while I dislike what the GCS mandate on these matters, we should
strive to be consistent anyway.
I trusted emacs...
+sub libfile ($)
+{
+ my ($f) = @_;
+ foreach (@userlibdir) {
+ return $_/$f
+ if -r $_/$f;
Wouldn't a `-f $_/$f' be more correct here?
Is there a case we want to know if a file exists but fail reading it?
Should we continue to search for another file? For me both are OK. But
I can change to -f
+ }
+ return $userlibdir[0] . / . $f;
Shouldn't this be `$libdir' instead of `$userlibdir[0]'? Otherwise,
No. $userlibdir[0] should contain the first directory in $libdir. If
it was not like that, then there is a problem.
Automake even fails to bootstrap itself, giving the error message:
Use of uninitialized value $userlibdir[0] in concatenation (.) or string at
./automake.tmp line 7088.
automake.tmp: error: cannot open /am/header-vars.am: No such file or
directory
I am not sure how you got this one.
With the change I'm proposing, Automake bootstrap itself *and* passes
the whole testsuite.
Is not it make distcheck enough? Because in my case it was working.
+ 'libdir=s' = sub { push @userlibdir,
+ (map { ($_ eq '') ? $libdir : $_ }
+ split (':', $_[1])); },
We should use $PATH_SEPARATOR here, not ':'.
OK
+ foreach (@userlibdir) {
+ my $dir = $_;
+ if (-d $dir/lang) {
+ opendir(LANG_DIR, $dir/lang) || next;
+ my @files=readdir(LANG_DIR);
+ foreach (@files) {
+ do $dir/lang/$_
+ if (-r $dir/lang/$_ $_ =~ /\.pm$/);
Wouldn't a `-f $dir/lang/$_' be more correct here?
To me, that is the same.
+user_def_language.test \
Typo; should be `user_def_lang.test'.
I am wondering if I sent you the right patch...
diff --git a/tests/user_def_lang.test b/tests/user_def_lang.test
new file mode 100755
index 000..9b015da
--- /dev/null
+++ b/tests/user_def_lang.test
This test is missing the #! /bin/sh shebang line, the copyright header, and
a brief comment explaining what is being tested.
+cat
Re: User extensions
On Sat, Oct 23, 2010 at 5:27 PM, Ralf Wildenhues ralf.wildenh...@gmx.de wrote: I haven't looked at the patch in detail yet, but will, now that the assignment papers are done (thanks!). It is done. -- Valentin David valentin.da...@gmail.com
Re: User extensions
Hi Valentin. Thank you very much for your patch implementing this much-desired automake feature. Here is a preliminary review... On Friday 03 September 2010, Valentin David wrote: Dear Automake list, I work with programming languages. It is very common to write small compilers, parser generators, or other. When using my tools, it is hard to convince my co-workers they should use the autotools when I cannot do the proper extension of Automake. Other people end up writing builders for Eclipse. I will never go to the dark side. And I wish to provide the right developing tools for my users, e.g. an extension to Automake. It is very easy to extend Autoconf for instance. But not for Automake. I realize, it is not often that people want to extend Automake. Most of the time, fancy libraries are coming with their Autoconf files, and the user is using the same compilers. Libraries do not need extension in Automake. Compilers do. To keep a patch set for Automake is not an option. It already irritates the system administrators when I tell them that the versions of the autotools are too old. Moreover, I cannot ask my co-workers to install it by hand in another prefix. You would surely ask me why I do not just write generic rules in a makefile to be included anywhere I need to use my tool. This has several problems. The first problem I encounter, is the dependency files. My tools might generate some. It is too much to ask a user to include by hand a dependency file. If there are several objects to compile, then they will just probably forget to include some. Also, you need to use -include (which I wonder whether it is portable), or trick Automake by using @am__include@. The latter might be caught by the config command from depout.m4, so you do not have to make one. Tell the users why they cannot use just include instead... Second problem, you cannot use the Automake variables to generate the right files. The user needs to trick. Here is an example. #INCDEP is subst to include ./$(DEPDIR)/ @inc...@foo.pmyhh EXTRA_DIST=foo.myhh foo_SOURCES=foo.cc nodist_foo_SOURCES=foo.hh # This does not work, guess why. foo-foo.o: foo.hh # This one will work, but it is like killing a # fly with the assault rifle, specially if you # have several objects. Hope you have ccache. $(foo_OBJECTS): foo.hh So now, it is not obvious for the user why foo.myhh is not a source, but an extra, why foo.hh has to be set manually in the sources, and you are happy when your user understands what nodist_ means. What I wanted the user to write was only: foo_SOURCES=foo.cc foo.myhh After all you can do it with Yacc or Lex, why not with my tools? I propose a patch as attached. * The lang_*_rewrite are added to the Language structure. The default is lang_sub_obj. They do not return anymore the object extension because the field 'output_extensions' already computes it. Hmm... looking at the git diffs, this seems like a useful refactoring by itself. Could this be done in separate, preparatory patch preceding the implementation of the user extensions support proper? * Compilers that generate source files might still generate dependency files. * --libdir= can be called several times, the arguments can also have a list of paths separated by a colon. We should really use the system PATH separator here, for better portability (such PATH separator is already computed and AC_SUBST'd by configure as `...@path_separator@'). Empty paths correspond to the original libdir. I guess this is done so that the user can override the previously-passed custom libdirs with the default one, without having to know its complete path -- right? I think this is useful and smart, but couldn't we find a better speacial value than empty to indicate the default libdir? * For all libdir, all libdir/lang/*.pm are loaded. This happens right after parsing the options. Clean and simple. I agree that this is the best way to go ATM. The test tests/user_def_lang.test should show how the feature works. We'll need much deeper test than the one provided by your patch (I can help writing them if you want). I can understand that loading perl files is not really nice because there easily can be problems of backward compatibility. But it's surely better than the present situation IMHO. I am waiting for your comments. Best regards, -*-*-*- I don't know the automake internals very well, so I cannot judge the overall, complete effect of your patch (which anyway *seems* quite limited -- which is good, and a point in favor of your patch). That said, I think I can provide some (hopefully) useful, albeit limited, criticism. diff --git a/automake.in b/automake.in index c0c5289..25c3467 100644 --- a/automake.in +++ b/automake.in @@ -107,7 +107,14 @@ struct (# Short name of the language (c, f77...). # If TRUE, nodist_ sources will be compiled using specific rules # (i.e. not
Re: User extensions
Hello Stefano, Valentin, please discuss patches on the -patches list; and please keep the original poster in Cc:, thanks. I haven't looked at the patch in detail yet, but will, now that the assignment papers are done (thanks!). * Stefano Lattarini wrote on Sat, Oct 23, 2010 at 04:05:02PM CEST: On Friday 03 September 2010, Valentin David wrote: * --libdir= can be called several times, the arguments can also have a list of paths separated by a colon. We should really use the system PATH separator here, for better portability (such PATH separator is already computed and AC_SUBST'd by configure as `...@path_separator@'). Why would an arbitrary path, having nothing to do with the PATH environment variable, needlessly have the same system-dependent separator? Other paths don't do this, either. Thanks, Ralf
Re: User extensions
On Saturday 23 October 2010, Ralf Wildenhues wrote: Hello Stefano, Valentin, please discuss patches on the -patches list; and please keep the original poster in Cc:, thanks. I haven't looked at the patch in detail yet, but will, now that the assignment papers are done (thanks!). * Stefano Lattarini wrote on Sat, Oct 23, 2010 at 04:05:02PM CEST: On Friday 03 September 2010, Valentin David wrote: * --libdir= can be called several times, the arguments can also have a list of paths separated by a colon. We should really use the system PATH separator here, for better portability (such PATH separator is already computed and AC_SUBST'd by configure as `...@path_separator@'). Why would an arbitrary path, having nothing to do with the PATH environment variable, needlessly have the same system-dependent separator? So that, on windows, one can do e.g. autoamke --libdir 'D:\my-am-libdir' -- or am I missing something? Other paths don't do this, either. Which kind of PATHs are you referring to? For example, PERL5LIB and PYTHONPATH both shares the PATH format (entries separated by : on Unix, by ; on Windows). Regards, Stefano
Re: User extensions
Hello Valentin, * Valentin David wrote on Fri, Sep 03, 2010 at 06:56:53PM CEST: I am waiting for your comments. well, I very much like the ideas in your mail. I didn't look at the patch yet, because you don't yet have copyright papers in place. Details off-list. Thanks, Ralf
Re: User extensions
Op 3 sep 2010, om 18:56 heeft Valentin David het volgende geschreven: I propose a patch as attached. You rule. thank you so much for this. :) -- k
