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
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
