Re: [Bug-wget] Remove most warnings for missing extern

[Tim Rühsen]

Hi Darshit,

the same problem on a different machine (though also Debian unstable).

I also ran
> >> make maintainer-clean
> >> ./bootstrap
> >> ./configure
> >> make

Maybe something is missing in your patches ?
Or a make bug ? (make 4.0-8)


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

../lib/libgnu.a IMHO is wrong.

wget_SOURCES includes build_info.c which depends on $(srcdir)Makefile.am and 
$(srcdir)/build_info.c.in.

Using these lines and everything works:

version.h: version.c
...
version.c: build_info.c
...

But version.h is static, so it should go into git and not being created.

Tim 

Am Freitag, 21. November 2014, 23:24:49 schrieb Darshit Shah:
> 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 ---

signature.asc
Description: This is a digitally signed message part.