Re: pg_regress: stat correct paths

From: Jorgen Austvik <Jorgen(dot)Austvik(at)Sun(dot)COM>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: pg_regress: stat correct paths
Date: 2007-11-26 17:27:29
Message-ID: 474B0201.4040405@sun.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-patches

Tom Lane wrote:
> Jorgen Austvik <Jorgen(dot)Austvik(at)Sun(dot)COM> writes:
>> Tom Lane wrote:
>>> Actually, it looks to me like the patch is wrong specifically because
>>> it does not do the right thing in the VPATH case.
>
>> Are you thinking about "failing if the folders are missing" as "not the
>> right thing in the VPATH case", or are you thinking about something else?
>
> The point is that in VPATH you are running in a build tree, and should
> copy the source files from the source tree, but *not* modify the source
> tree. Thus, fetching relative to $srcdir but writing relative to .
> is in fact the correct behavior.

Ah, I understand.

It is this part you don't like:
---8<--------------8<--------------8<--------------8<--------------8<-----------
+ snprintf(destdir, MAXPGPATH, "%s/%s", abs_srcdir, dest);
<snip>
! snprintf(destfile, MAXPGPATH, "%s/%s.%s", destdir, prefix, suffix);
---8<--------------8<--------------8<--------------8<--------------8<-----------

Thanks for the guidance, I'll try come up with a better alternative.
Should outputdir be used instead of current working directory (cwd) if
it is set?

> There has not previously been any complaint that pg_regress was broken
> in this regard, so maybe you should take two steps back and explain what
> problem you think needs fixing, rather than just dropping a patch on us.

I tried to explain it in the mail, but let me try again, this time
showing some code.

Here we stat <cwd>/input:

---8<--------------8<--------------8<--------------8<--------------8<-----------
static void
convert_sourcefiles(void)
{
struct stat st;
int ret;

ret = stat("input", &st);
if (ret == 0 && S_ISDIR(st.st_mode))
convert_sourcefiles_in("input", "sql", "sql");
---8<--------------8<--------------8<--------------8<--------------8<-----------

But if we have set srcdir, the directory we are stat'ing, is not the
same directory that we are reading the files from:

---8<--------------8<--------------8<--------------8<--------------8<-----------
static void
convert_sourcefiles_in(char *source, char *dest, char *suffix)

<snip>

if (srcdir)
strcpy(abs_srcdir, srcdir);
else
strcpy(abs_srcdir, abs_builddir);

snprintf(indir, MAXPGPATH, "%s/%s", abs_srcdir, source);
names = pgfnames(indir);
---8<--------------8<--------------8<--------------8<--------------8<-----------

So I wanted to provide a patch that ran stat on the folder that we were
reading the files from.

-J
--
Jørgen Austvik, Software Engineering
Database Technology Group

In response to

Responses

Browse pgsql-patches by date

  From Date Subject
Next Message Tom Lane 2007-11-26 17:42:21 Re: pg_regress: stat correct paths
Previous Message Tom Lane 2007-11-26 17:10:12 Re: pg_regress: stat correct paths