headers
Michael Snyder | 1 Jan 2011 01:19
Favicon

Re: [patch] more comment cleanups

H.J. Lu wrote:
> On Fri, Dec 31, 2010 at 3:12 PM, Mark Kettenis <mark.kettenis <at> xs4all.nl> wrote:
>>> Date: Fri, 31 Dec 2010 15:00:48 -0800
>>> From: Michael Snyder <msnyder <at> vmware.com>
>>>
>>> Also break up some long lines.
>> Sorry Michael, but I don't think breaking up lines like that actually
>> improves the readability of the code.  Many of those fit comfrotably
>> in standard 80-column display.  By breaking them up they take up more
>> vertical space, which means that I can see less code.
>>
> 
> FWIW, I prefer 72 column to show line numbers in 80 column terminal.

When we get new contributions, we don't allow them to go out to 80 
columns.  I'm just trying to apply the same standard to the old code.

But maybe I did go a bit overboard on some of this.  I'll try to be more 
restrained.

I'm being guided by where eMacs breaks up lines when I format a comment,
which is something like 72 columns.
Permalink | Reply | 16 comments |
headers
Jan Kratochvil | 1 Jan 2011 02:13
Picon
Favicon

[patch] make info regression on --with-system-readline

Hi,

since:
	[2/2] RFA: --with-system-readline -vs- gdb.texinfo
	http://sourceware.org/ml/gdb-patches/2010-11/msg00270.html

$ rm -rf gdb-7.2.50.20101231; tar xjf gdb-7.2.50.20101231.tar.bz2; cd gdb-7.2.50.20101231; CFLAGS=
./configure --with-system-readline; make; rm gdb/doc/gdb.info; make -C gdb/doc gdb.info
->
[...]
makeinfo  -I ./../mi -I . \
	-o gdb.info ./gdb.texinfo
./gdb.texinfo:30521:  <at> include `rluser.texi': No such file or directory.
./gdb.texinfo:30521:  <at> include `inc-hist.texinfo': No such file or directory.
[...]
Fedora 14 x86_64

It is because GDBvn.texi has started to depend on the configure options.

I will check it in in the case of no comments.

Another issue is that GDBvn.texi and gdb-cfg.texi should not be distributed.
But that bug is a different one on top of this bug.  That bug of a needless
files distribution is dependent on magic GDB `make dist' I do not know and
also that dist bug is not serious enough.

This is a regression.

Thanks,
Jan
(Continue reading)

Permalink | Reply | 21 comments |
headers
Jan Kratochvil | 1 Jan 2011 09:08
Picon
Favicon

[patch] make info out-of-src-tree regression on --with-system-readline

On Sat, 01 Jan 2011 02:13:24 +0100, Jan Kratochvil wrote:
> Another issue is that GDBvn.texi and gdb-cfg.texi should not be distributed.
> But that bug is a different one on top of this bug.  That bug of a needless
> files distribution is dependent on magic GDB `make dist' I do not know and
> also that dist bug is not serious enough.

This part has a real consequence - the previous patch does not apply for build
trees out of the src tree.  It may be even a makeinfo bug.

The change below is not needed for texi2dvi.  In fact it even breaks it.

As Fedora does not package texi2roff I did not test it.

Another possibility is to error out if $(srcdir)/GDBvn.texi exists so that no
disambiguities may exist.  GDBvn.texi would have to be removed from the
distributed tars.

Comment on this part is more than welcome.

This is a regression.

Thanks,
Jan

gdb/doc/
2011-01-01  Jan Kratochvil  <jan.kratochvil <at> redhat.com>

	Fix out-of-src doc build if using --with-system-readline.
	* Makefile.in (MAKEINFO): New comment.
	(gdb.info, gdb/index.html): Remove $(srcdir)/ from gdb.texinfo.
(Continue reading)

Permalink | Reply | 2 comments |
headers
Eli Zaretskii | 1 Jan 2011 10:12
Picon

Re: [patch] more comment cleanups

> Date: Fri, 31 Dec 2010 16:19:58 -0800
> From: Michael Snyder <msnyder <at> vmware.com>
> CC: Mark Kettenis <mark.kettenis <at> xs4all.nl>,  "gdb-patches <at> sourceware.org" <gdb-patches <at> sourceware.org>
> 
> I'm being guided by where eMacs breaks up lines when I format a comment,
> which is something like 72 columns.

Actually, it's 70 columns by default.
Permalink | Reply | 15 comments |
headers
Eli Zaretskii | 1 Jan 2011 10:25
Picon

Re: [patch] make info regression on --with-system-readline

> Date: Sat, 1 Jan 2011 02:13:24 +0100
> From: Jan Kratochvil <jan.kratochvil <at> redhat.com>
> Cc: Tom Tromey <tromey <at> redhat.com>
> 
> since:
> 	[2/2] RFA: --with-system-readline -vs- gdb.texinfo
> 	http://sourceware.org/ml/gdb-patches/2010-11/msg00270.html
> 
> $ rm -rf gdb-7.2.50.20101231; tar xjf gdb-7.2.50.20101231.tar.bz2; cd gdb-7.2.50.20101231; CFLAGS=
./configure --with-system-readline; make; rm gdb/doc/gdb.info; make -C gdb/doc gdb.info
> ->
> [...]
> makeinfo  -I ./../mi -I . \
> 	-o gdb.info ./gdb.texinfo
> ./gdb.texinfo:30521:  <at> include `rluser.texi': No such file or directory.
> ./gdb.texinfo:30521:  <at> include `inc-hist.texinfo': No such file or directory.
> [...]
> Fedora 14 x86_64
> 
> It is because GDBvn.texi has started to depend on the configure options.

Aren't you supposed to "make distclean" whenever you reconfigure?
That's what I do in every project, because I don't trust the Makefiles
to DTRT in such case.  E.g., what about all the *.o files you didn't
remove?

> -GDBvn.texi : ${gdbdir}/version.in
> +GDBvn.texi : ${gdbdir}/version.in Makefile

Thanks.
(Continue reading)

Permalink | Reply | 17 comments |
headers
Eli Zaretskii | 1 Jan 2011 10:36
Picon

Re: [patch] make info out-of-src-tree regression on --with-system-readline

> Date: Sat, 1 Jan 2011 09:08:48 +0100
> From: Jan Kratochvil <jan.kratochvil <at> redhat.com>
> Cc: Tom Tromey <tromey <at> redhat.com>
> 
> On Sat, 01 Jan 2011 02:13:24 +0100, Jan Kratochvil wrote:
> > Another issue is that GDBvn.texi and gdb-cfg.texi should not be distributed.
> > But that bug is a different one on top of this bug.  That bug of a needless
> > files distribution is dependent on magic GDB `make dist' I do not know and
> > also that dist bug is not serious enough.
> 
> This part has a real consequence - the previous patch does not apply for build
> trees out of the src tree.  It may be even a makeinfo bug.
> 
> The change below is not needed for texi2dvi.  In fact it even breaks it.

Can't you repair it by a suitable setting of TEXINPUTS?

> Another possibility is to error out if $(srcdir)/GDBvn.texi exists so that no
> disambiguities may exist.  GDBvn.texi would have to be removed from the
> distributed tars.

What are the problems with not distributing it in the tarball, again?
(I take back my questions in the previous mail.)

>  gdb.info: ${GDB_DOC_FILES}
>  	$(MAKEINFO) $(READLINE_TEXI_INCFLAG) -I ${GDBMI_DIR} -I $(srcdir) \
> -		-o gdb.info $(srcdir)/gdb.texinfo
> +		-o gdb.info gdb.texinfo

If we put "-I ." _before_ "-I $(srcdir)", doesn't it solve the issue
(Continue reading)

Permalink | Reply | 1 comment |
headers
Jan Kratochvil | 1 Jan 2011 10:38
Picon
Favicon

Re: [patch] make info regression on --with-system-readline

On Sat, 01 Jan 2011 10:25:21 +0100, Eli Zaretskii wrote:
> Aren't you supposed to "make distclean" whenever you reconfigure?

In normal projects I am not used to.  In GDB I do "make clean" but it may not
be fully reliable, I believe there is more broken in GDB build system.

But even if you do just first configure it is now broken in GDB as the files
get inappropriately distributed.

> E.g., what about all the *.o files you didn't remove?

They depend on config.h which gets regenerated.

Maybe if you only change CFLAGS and config.h stays the same (and it preserves
its timestamp). you would need `make clean'.

> However, I don't like rules that depend of Makefiles, because they
> tend to be re-run too much for no good reason.  Note that this will
> re-make the docs each time you reconfigure, even if you didn't change
> the configuration.

We can stamp etc. GDBvn.texi if it is a concern (I do not find it so).

> > Another issue is that GDBvn.texi and gdb-cfg.texi should not be distributed.
> 
> How can we not distribute them when gdb.texinfo  <at> include's them, and
> needs that for setting some of the variables the manual uses?  If we
> don't distribute them, end users will be unable to rebuild the manual.
> What am I missing here?
(Continue reading)

Permalink | Reply | 16 comments |
headers
Eli Zaretskii | 1 Jan 2011 11:02
Picon

Re: [patch] make info regression on --with-system-readline

> Date: Sat, 1 Jan 2011 10:38:15 +0100
> From: Jan Kratochvil <jan.kratochvil <at> redhat.com>
> Cc: gdb-patches <at> sourceware.org, tromey <at> redhat.com
> 
> On Sat, 01 Jan 2011 10:25:21 +0100, Eli Zaretskii wrote:
> > Aren't you supposed to "make distclean" whenever you reconfigure?
> 
> In normal projects I am not used to.  In GDB I do "make clean" but it may not
> be fully reliable, I believe there is more broken in GDB build system.

"make distclean" _should_ be reliable, since its main raison d'etre is
to allow a clean build after re-configuration.  So maybe we should fix
that instead (e.g., it doesn't currently remove GDBvn.texi).

> But even if you do just first configure it is now broken in GDB as the files
> get inappropriately distributed.

Sorry, I don't follow.  Can you describe the scenario in more detail?

> > E.g., what about all the *.o files you didn't remove?
> 
> They depend on config.h which gets regenerated.

Ah, yes, that nuisance.  I remember complaining about that a few years
ago, but my opinions were voted down.

> Maybe if you only change CFLAGS and config.h stays the same (and it preserves
> its timestamp). you would need `make clean'.

But that's just it: without a very thorough examination of the
(Continue reading)

Permalink | Reply | 15 comments |
headers
Jan Kratochvil | 1 Jan 2011 11:00
Picon
Favicon

Re: [patch] make info out-of-src-tree regression on --with-system-readline

On Sat, 01 Jan 2011 10:36:47 +0100, Eli Zaretskii wrote:
> > The change below is not needed for texi2dvi.  In fact it even breaks it.
> 
> Can't you repair it by a suitable setting of TEXINPUTS?

It does not work for me for makeinfo as tested now.

> What are the problems with not distributing it in the tarball, again?

If you can test it then it would be great.  My attempt with `-f src-release'
failed.  I did not try hard.

Still I would find it fragile to generate file in . and search for it first in
$(srcdir).

> >  gdb.info: ${GDB_DOC_FILES}
> >  	$(MAKEINFO) $(READLINE_TEXI_INCFLAG) -I ${GDBMI_DIR} -I $(srcdir) \
> > -		-o gdb.info $(srcdir)/gdb.texinfo
> > +		-o gdb.info gdb.texinfo
> 
> If we put "-I ." _before_ "-I $(srcdir)", doesn't it solve the issue
> more nicely?

I tried first but it does not work.  I did not check the makeinfo sources but
still GDB should probably workaround it even if it is a makeinfo bug.

Thanks,
Jan
(Continue reading)

Permalink | Reply | 0 comments |
headers
Jan Kratochvil | 1 Jan 2011 12:16
Picon
Favicon

Re: [patch] make info regression on --with-system-readline

On Sat, 01 Jan 2011 11:02:02 +0100, Eli Zaretskii wrote:
> "make distclean" _should_ be reliable, since its main raison d'etre is
> to allow a clean build after re-configuration.  So maybe we should fix
> that instead (e.g., it doesn't currently remove GDBvn.texi).

If "make distclean" should clean it then it also should not be distributed.
And we are back at the point GDB currently cannot do easily `make dist'.

> > But even if you do just first configure it is now broken in GDB as the files
> > get inappropriately distributed.
> 
> Sorry, I don't follow.  Can you describe the scenario in more detail?

rm -rf gdb-7.2.50.20101231; tar xjf gdb-7.2.50.20101231.tar.bz2; cd gdb-7.2.50.20101231; patch -p1
<THE-FIRST-PATCH-POSTED; mkdir b; cd b; CFLAGS= ../configure --with-system-readline; make; rm
gdb/doc/gdb.info; make -C gdb/doc gdb.info
../../../gdb/doc/gdb.texinfo:30521:  <at> include `rluser.texi': No such file or directory.
../../../gdb/doc/gdb.texinfo:30521:  <at> include `inc-hist.texinfo': No such file or directory.

Both exist:
./gdb-7.2.50.20101231/gdb/doc/GDBvn.texi
./gdb-7.2.50.20101231/b/gdb/doc/GDBvn.texi

> I prefer to do with GDBvn.texi what many projects do with config.h:
> regenerate it on a temporary file, then use move-if-change to move it
> into the real file.  Would that resolve your problem?  It certainly
> resolves mine.

OK, going to post it.
(Continue reading)

Permalink | Reply | 12 comments |

Gmane