>IMHO, version.c and version.h has wrong dependencies in Makefile.am.
>They depend on changed wget_SOURCES, which don't have to change.
I think version.c and version.h have the right dependencies. I'm not sure
about the dependency on ../lib/libgnu.a, but the dependency on wget_SOURCES
is perfect.
On 11/21, Tim Rühsen wrote:
>On Friday 21 November 2014 19:58:46 Darshit Shah wrote:
>> On 11/21, Tim Rühsen wrote:
>> >On Friday 21 November 2014 17:13:22 Darshit Shah wrote:
>> >> Clang provides some warnings for missing extern declarations for
>> >> non-static
>> >> variables. The following two patches clear most of them. I can
>> >> currently
>> >> see only more such warning which is caused by build_info.c. To fix
>> >> this,
>> >> someone will have to hack on the build_info.px perl script.
>> >
>> >After ./boostrap
>> >./configure
>> >
>> >I get
>> >
>> >main.c:58:21: fatal error: version.h: No such file or directory
>> >
>> > #include "version.h"
>>
>> I just ran:
>> make maintainer-clean
>> ./bootstrap
>> ./configure
>> make
>>
>> and it works for me without any issues. Maybe you could try running a
>> simple make clean first? If it still doesn't work, I'll try looking into
>> it again. But since I can't reproduce it, its hard to guess where the
>> error would be.>
>Try to remove src/version.c and src/version.h. Than perform a 'make'.
I confirmed that both version.c and version.h didn't exist when I ran make.
I also tried again by simply by deleting version.* before running make
again. It still compiles perfectly for me.
>IMHO, version.c and version.h has wrong dependencies in Makefile.am.
>They depend on changed wget_SOURCES, which don't have to change.
I think version.c and version.h have the right dependencies. I'm not sure
about the dependency on ../lib/libgnu.a, but the dependency on wget_SOURCES
is perfect.
Let me start explaining my reasons with a question, when does version.c need
to be re-created?
Version.c needs regeneration when any of its component strings have a change
in value.
The $COMPILE string changes when the libraries being used change, or more
frequently, when the CFLAGS variable changes.
The link_string is dependent on the AM_CFLAGS, CFLAGS, LDFLAGS, AM_LDFLAGS
and LIBS variables.
One last part that the version data is dependent on is the current patch
level of Wget.
Now, within a Makefile, how do we determine these dependencies? Now, I'm not
sure if commits that solely touch the non-C source files cause version.c to
be regenerated. But all other commits will change files in wget_SOURCES.
Hence forcing a change in version.c
Similarly, any change in the CFLAGS, LDFLAGS or LIBS variables will also
cause files in the wget_SOURCES variable to be recompiled. As a result of
that, version.c will be marked for re-compilation since its dependencies
have changed. This is exactly how we want it to behave.
>I could make it work by using
>version.h: $(srcdir)/Makefile.am
>...
>version.c: version.h
>
>This at least worked (after a ./config.status), but it may be wrong.
>What exactly has to change so that we need to rebuild version.[ch] ?
>It's not the Wget sources !
In general, version.h doesn't need to be rebuilt. I think the best way is to
explicitly write it and commit version.h into the source tree. It doesn't
depend on any values that are generated at runtime. If everyone agrees on
this setup, I'll gladly change the patch to simply statically include
version.h. This does seem like a better and more efficient idea to me.
version.c on the other hand, does indeed need to be re-generated every time
the wget_SOURCES are regenerated. Because it marks a change in either the
patch level, or in one of the compilation variables which is other
impossible to track from a Makefile.
>Tim
>
>> >Maybe you can also fix these warnings them before pushing:
>> >
>> >host.c: In function 'address_list_set_faulty':
>> >host.c:148:55: warning: unused parameter 'index' [-Wunused-parameter]
>> >
>> > address_list_set_faulty (struct address_list *al, int index)
>> >
>> >cookies.c: In function 'discard_matching_cookie':
>> >cookies.c:303:15: warning: variable 'res' set but not used
>> >[-Wunused-but-set- variable]
>> >
>> > int res;
>>
>> It seems these warnings aren't enabled by -Wall and -Wextra on Clang. In
>> fact -Wunused-but-set-variable is GCC specific only. Which is why I never
>> saw these warnings in my output.
>
>There is also:
>utils.c: In function 'abort_run_with_timeout':
>utils.c:1919:29: warning: unused parameter 'sig' [-Wunused-parameter]
>
> abort_run_with_timeout (int sig)
--- end quoted text ---
pgpl7WnYrHmbI.pgp