Re: vpath builds and verbose error messages

When using verbose error messages (psql \set VERBOSITY verbose) with a
vpath build, you get this sort of thing:

ERROR: 42703: column "foo" does not exist
LINE 1: select foo;
^
LOCATION: transformColumnRef, /build/buildd-postgresql-9.1_9.1.1-3-i386-AP0ovQ/postgresql-9.1-9.1.1/build/../src/backend/parser/parse_expr.c:766

Can we shorten that path somehow? Either in the C code when printing it
out, or during the build. Any ideas?

Excerpts from Peter Eisentraut's message of vie nov 18 01:34:18 -0300 2011:
> When using verbose error messages (psql \set VERBOSITY verbose) with a
> vpath build, you get this sort of thing:
>
> ERROR: 42703: column "foo" does not exist
> LINE 1: select foo;
> ^
> LOCATION: transformColumnRef, /build/buildd-postgresql-9.1_9.1.1-3-i386-AP0ovQ/postgresql-9.1-9.1.1/build/../src/backend/parser/parse_expr.c:766
>
> Can we shorten that path somehow? Either in the C code when printing it
> out, or during the build. Any ideas?

Huh, I hadn't noticed, even though I see those all the time. I agree
with shortening the path so that it's relative to the root source dir,
if that's what you propose:

LOCATION: transformColumnRef, src/backend/parser/parse_expr.c:766

If it can be done at build time that would be best, because we wouldn't
be carrying thousands of useless copies of the source path ... I have
no suggestions on how to do this, however.

--
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> Excerpts from Peter Eisentraut's message of vie nov 18 01:34:18 -0300 2011:
>> When using verbose error messages (psql \set VERBOSITY verbose) with a
>> vpath build, you get this sort of thing:
>> LOCATION: transformColumnRef, /build/buildd-postgresql-9.1_9.1.1-3-i386-AP0ovQ/postgresql-9.1-9.1.1/build/../src/backend/parser/parse_expr.c:766
>>
>> Can we shorten that path somehow? Either in the C code when printing it
>> out, or during the build. Any ideas?

> Huh, I hadn't noticed, even though I see those all the time. I agree
> with shortening the path so that it's relative to the root source dir,
> if that's what you propose:

In a non-vpath build you just get the file name (or at least that's what
I'm used to seeing). I agree with Peter that a full path seems a bit
over the top.

It wouldn't be that hard for elog.c to do strrchr(fname, '/') or
something like that, but the idea that there are hundreds of full-path
strings embedded in the executable is a bit annoying. I guess we could
hope that the compiler is bright enough to store it only once per source
file, so the actual space requirement may not be all *that* bad.

regards, tom lane

On fre, 2011-11-18 at 09:44 -0500, Tom Lane wrote:
> It wouldn't be that hard for elog.c to do strrchr(fname, '/') or
> something like that, but the idea that there are hundreds of full-path
> strings embedded in the executable is a bit annoying. I guess we
> could
> hope that the compiler is bright enough to store it only once per
> source
> file, so the actual space requirement may not be all *that* bad.

A look a the output of "strings" shows that the compiler does appear to
optimize this. So your suggestion is probably the way to go.

One thing that isn't so nice about all this is that it embeds the
personal directory structure of the builder of the binary into the
shipped product. But gcc's cpp doesn't like redefining __FILE__, so the
only way to get around that altogether would be to use something other
than __FILE__ and define that for all builds. Might not be worth it.

Excerpts from Peter Eisentraut's message of mar nov 22 16:22:15 -0300 2011:

> One thing that isn't so nice about all this is that it embeds the
> personal directory structure of the builder of the binary into the
> shipped product. But gcc's cpp doesn't like redefining __FILE__, so the
> only way to get around that altogether would be to use something other
> than __FILE__ and define that for all builds. Might not be worth it.

This is similar to what's suggested here:
http://stackoverflow.com/questions/1591873/how-do-i-write-a-cpp-dir-macro-similar-to-file

--
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> One thing that isn't so nice about all this is that it embeds the
> personal directory structure of the builder of the binary into the
> shipped product. But gcc's cpp doesn't like redefining __FILE__, so the
> only way to get around that altogether would be to use something other
> than __FILE__ and define that for all builds. Might not be worth it.

Well, if you have a problem with that, don't use a vpath build. I don't
think it's our job to work around gcc behaviors that someone else might
feel to be security issues --- they should take that up with the gcc
maintainers. To me, the only argument for doing anything about this at
all is that we'd like Postgres' behavior (in terms of what it prints in
error messages) to be consistent across different build scenarios.

regards, tom lane

On fre, 2011-11-18 at 09:44 -0500, Tom Lane wrote:
> It wouldn't be that hard for elog.c to do strrchr(fname, '/') or
> something like that,

Here is a patch for that. I would also like to backpatch this.

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> On fre, 2011-11-18 at 09:44 -0500, Tom Lane wrote:
>> It wouldn't be that hard for elog.c to do strrchr(fname, '/') or
>> something like that,

> Here is a patch for that. I would also like to backpatch this.

Hmmm ... is it possible that strrchr could change errno? If so we'd
need to re-order the operations a bit. Otherwise this seems fine.

regards, tom lane

On lör, 2011-11-26 at 10:45 -0500, Tom Lane wrote:
> Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> > On fre, 2011-11-18 at 09:44 -0500, Tom Lane wrote:
> >> It wouldn't be that hard for elog.c to do strrchr(fname, '/') or
> >> something like that,
>
> > Here is a patch for that. I would also like to backpatch this.
>
> Hmmm ... is it possible that strrchr could change errno?

It's not documented to do that, and an implementation would have to go
far out of it's way to do it anyway.