Enforce creation of destination folders for source files in pg_regress (Was: pg_regress writes into source tree)
Hi all,
pg_regress will fail with test suites using only source files if the
destination folders do not exist in the code tree. This is annoying
because this forces to maintain empty folders sql/ and expected/ with
a .gitignore ignoring everything. The issue has been discussed here
(/messages/by-id/54935D9D.7010608@dunslane.net)
but nobody actually sent a patch, so here is one, and a thread for
this discussion.
Regards,
--
Michael
Attachments:
20150115_pg_regress_create_destdir.patchtext/x-patch; charset=US-ASCII; name=20150115_pg_regress_create_destdir.patchDownload+11-2
On 1/14/15 11:31 PM, Michael Paquier wrote:
pg_regress will fail with test suites using only source files if the
destination folders do not exist in the code tree. This is annoying
because this forces to maintain empty folders sql/ and expected/ with
a .gitignore ignoring everything.
We'd still need the .gitignore files somewhere. Do you want to move
them one directory up?
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Feb 20, 2015 at 5:50 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
On 1/14/15 11:31 PM, Michael Paquier wrote:
pg_regress will fail with test suites using only source files if the
destination folders do not exist in the code tree. This is annoying
because this forces to maintain empty folders sql/ and expected/ with
a .gitignore ignoring everything.We'd still need the .gitignore files somewhere. Do you want to move
them one directory up?
I am not sure I am getting what you are pointing to... For extensions
that already have non-empty sql/ and expected/, they should have their
own ignore entries as sql/.gitignore and expected/.gitignore. The
point of the patch is to simplify the code tree of extensions that
need to keep empty sql/ and expected/, for example to be able to run
regression tests after a fresh repository clone for example.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2/20/15 1:56 AM, Michael Paquier wrote:
We'd still need the .gitignore files somewhere. Do you want to move
them one directory up?I am not sure I am getting what you are pointing to... For extensions
that already have non-empty sql/ and expected/, they should have their
own ignore entries as sql/.gitignore and expected/.gitignore. The
point of the patch is to simplify the code tree of extensions that
need to keep empty sql/ and expected/, for example to be able to run
regression tests after a fresh repository clone for example.
The affected modules have sql/.gitignore and/or expected/.gitignore
files, so the case that the directory doesn't exist and needs to be
created doesn't actually happen.
You could argue that these .gitignore files don't actually belong there,
but your patch doesn't change or move those files, and even modules that
have non-empty sql/ or expected/ directories have .gitignore files
there, so it is considered the appropriate location.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, Feb 21, 2015 at 6:51 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
On 2/20/15 1:56 AM, Michael Paquier wrote:
We'd still need the .gitignore files somewhere. Do you want to move
them one directory up?I am not sure I am getting what you are pointing to... For extensions
that already have non-empty sql/ and expected/, they should have their
own ignore entries as sql/.gitignore and expected/.gitignore. The
point of the patch is to simplify the code tree of extensions that
need to keep empty sql/ and expected/, for example to be able to run
regression tests after a fresh repository clone for example.The affected modules have sql/.gitignore and/or expected/.gitignore
files, so the case that the directory doesn't exist and needs to be
created doesn't actually happen.
What about extensions that do not have sql/ and extended/ in their
code tree? I don't have an example of such extensions in my mind but I
cannot assume either that they do not exist. See for example something
that happended a couple of months back, dummy_seclabel has been
trapped by that with df761e3, fixed afterwards with 3325624 (the
structure has been changed again since, but that's the error showed up
because of those missing sql/ and expected/).
You could argue that these .gitignore files don't actually belong there,
but your patch doesn't change or move those files, and even modules that
have non-empty sql/ or expected/ directories have .gitignore files
there, so it is considered the appropriate location.
This is up to the maintainer of each extension to manage their code
tree. However I can imagine that some people would be grateful if we
allow them to not need sql/ and expected/ containing only one single
.gitignore file ignoring everything with "*", making the code tree of
their extensions cleaner.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2/22/15 5:41 AM, Michael Paquier wrote:
You could argue that these .gitignore files don't actually belong there,
but your patch doesn't change or move those files, and even modules that
have non-empty sql/ or expected/ directories have .gitignore files
there, so it is considered the appropriate location.This is up to the maintainer of each extension to manage their code
tree. However I can imagine that some people would be grateful if we
allow them to not need sql/ and expected/ containing only one single
.gitignore file ignoring everything with "*", making the code tree of
their extensions cleaner.
I would like to have an extension in tree that also does this, so we
have a regression test of this functionality.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Feb 23, 2015 at 12:00 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
On 2/22/15 5:41 AM, Michael Paquier wrote:
You could argue that these .gitignore files don't actually belong there,
but your patch doesn't change or move those files, and even modules that
have non-empty sql/ or expected/ directories have .gitignore files
there, so it is considered the appropriate location.This is up to the maintainer of each extension to manage their code
tree. However I can imagine that some people would be grateful if we
allow them to not need sql/ and expected/ containing only one single
.gitignore file ignoring everything with "*", making the code tree of
their extensions cleaner.I would like to have an extension in tree that also does this, so we
have a regression test of this functionality.
Sure. Here is one in the patch attached added as a test module. The
name of the module is regress_dynamic. Perhaps the name could be
better..
--
Michael
Attachments:
0001-Enforce-creation-of-input-and-output-paths-in-pg_reg.patchtext/x-diff; charset=US-ASCII; name=0001-Enforce-creation-of-input-and-output-paths-in-pg_reg.patchDownload+10-2
0002-Add-regress_dynamic-as-a-test-module.patchtext/x-diff; charset=US-ASCII; name=0002-Add-regress_dynamic-as-a-test-module.patchDownload+63-1
On Feb 22, 2015, at 5:41 AM, Michael Paquier <michael.paquier@gmail.com> wrote:
This is up to the maintainer of each extension to manage their code
tree. However I can imagine that some people would be grateful if we
allow them to not need sql/ and expected/ containing only one single
.gitignore file ignoring everything with "*", making the code tree of
their extensions cleaner.
I don't see this as an improvement. What's wrong with a .gitignore file ignoring everything?
...Robert
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Feb 23, 2015 at 9:31 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Feb 22, 2015, at 5:41 AM, Michael Paquier <michael.paquier@gmail.com>
wrote:
This is up to the maintainer of each extension to manage their code
tree. However I can imagine that some people would be grateful if we
allow them to not need sql/ and expected/ containing only one single
.gitignore file ignoring everything with "*", making the code tree of
their extensions cleaner.I don't see this as an improvement. What's wrong with a .gitignore file
ignoring everything?
That's mainly a matter of code maintenance style (and I am on the side that
a minimum number of folders in a git tree makes things easier to follow),
and IMO it is strange to not have some flexibility. Btw, the reason why
this patch exists is this thread of last December:
/messages/by-id/20141212134508.GT1768@alvh.no-ip.org
--
Michael
On 2/23/15 1:27 AM, Michael Paquier wrote:
I would like to have an extension in tree that also does this, so we
have a regression test of this functionality.
Sure. Here is one in the patch attached added as a test module. The
name of the module is regress_dynamic. Perhaps the name could be
better..
I was more thinking like we could use an existing module like file_fdw.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2/24/15 1:23 AM, Michael Paquier wrote:
On Mon, Feb 23, 2015 at 9:31 PM, Robert Haas <robertmhaas@gmail.com
<mailto:robertmhaas@gmail.com>> wrote:On Feb 22, 2015, at 5:41 AM, Michael Paquier <michael.paquier@gmail.com <mailto:michael.paquier@gmail.com>> wrote:
This is up to the maintainer of each extension to manage their code
tree. However I can imagine that some people would be grateful if we
allow them to not need sql/ and expected/ containing only one single
.gitignore file ignoring everything with "*", making the code tree of
their extensions cleaner.I don't see this as an improvement. What's wrong with a .gitignore
file ignoring everything?That's mainly a matter of code maintenance style (and I am on the side
that a minimum number of folders in a git tree makes things easier to
follow), and IMO it is strange to not have some flexibility. Btw, the
reason why this patch exists is this thread of last December:
/messages/by-id/20141212134508.GT1768@alvh.no-ip.org
What's an example of this? I ask because what I'd like is to break the
cluster management portion of pg_regress out on it's own to make it easy
to get that functionality while using a different test framework (like
pgTap).
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Feb 25, 2015 at 4:27 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
On 2/23/15 1:27 AM, Michael Paquier wrote:
I would like to have an extension in tree that also does this, so we
have a regression test of this functionality.
Sure. Here is one in the patch attached added as a test module. The
name of the module is regress_dynamic. Perhaps the name could be
better..I was more thinking like we could use an existing module like file_fdw.
Right. I forgot this one. A patch is attached to do so.
--
Michael
Attachments:
0001-Enforce-creation-of-input-and-output-paths-in-pg_reg.patchtext/x-diff; charset=US-ASCII; name=0001-Enforce-creation-of-input-and-output-paths-in-pg_reg.patchDownload+10-2
0002-Remove-expected-and-sql-in-file_fdw.patchtext/x-diff; charset=US-ASCII; name=0002-Remove-expected-and-sql-in-file_fdw.patchDownload+2-3
* Michael Paquier (michael.paquier@gmail.com) wrote:
On Wed, Feb 25, 2015 at 4:27 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
On 2/23/15 1:27 AM, Michael Paquier wrote:
I would like to have an extension in tree that also does this, so we
have a regression test of this functionality.
Sure. Here is one in the patch attached added as a test module. The
name of the module is regress_dynamic. Perhaps the name could be
better..I was more thinking like we could use an existing module like file_fdw.
Right. I forgot this one. A patch is attached to do so.
I'm always doing VPATH builds, but I believe there's still an
outstanding question on this from Peter about if we need to add entries
to .gitignore for, eg, sql/file_fdw.sql if these directories don't exist
because otherwise those would show up to someone doing a git status and
to Robert's point- if we need those anyway, it's probably better to have
them be in those directories rather than up a level and referring to
directories that don't exist in the tree.
Having the extra directories already exist doesn't seem like a bad thing
to me anyway. If they are causing confusion or something then perhaps a
README could be added to let people know why they exist.
I'm going to mark this back to 'waiting on author' in case there's
something material that I've missed which you can clarify. I had
started this review thinking to help move it along but after re-reading
the thread and thinking about it for a bit, I came around to the other
side and therefore think it should probably be rejected. We certainly
appreciate the effort, of course.
Thanks!
Stephen
On Sun, Mar 1, 2015 at 2:38 AM, Stephen Frost <sfrost@snowman.net> wrote:
I'm going to mark this back to 'waiting on author' in case there's
something material that I've missed which you can clarify. I had
started this review thinking to help move it along but after re-reading
the thread and thinking about it for a bit, I came around to the other
side and therefore think it should probably be rejected. We certainly
appreciate the effort, of course.
Re-reading this thread there are 3 perplexed opinions from three
different committers, and nobody pleading in favor of this patch
except me, so let's simply mark this as rejected and move on.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
* Michael Paquier (michael.paquier@gmail.com) wrote:
On Sun, Mar 1, 2015 at 2:38 AM, Stephen Frost <sfrost@snowman.net> wrote:
I'm going to mark this back to 'waiting on author' in case there's
something material that I've missed which you can clarify. I had
started this review thinking to help move it along but after re-reading
the thread and thinking about it for a bit, I came around to the other
side and therefore think it should probably be rejected. We certainly
appreciate the effort, of course.Re-reading this thread there are 3 perplexed opinions from three
different committers, and nobody pleading in favor of this patch
except me, so let's simply mark this as rejected and move on.
Works for me.
Thanks!
Stephen
Michael Paquier wrote:
On Sun, Mar 1, 2015 at 2:38 AM, Stephen Frost <sfrost@snowman.net> wrote:
I'm going to mark this back to 'waiting on author' in case there's
something material that I've missed which you can clarify. I had
started this review thinking to help move it along but after re-reading
the thread and thinking about it for a bit, I came around to the other
side and therefore think it should probably be rejected. We certainly
appreciate the effort, of course.Re-reading this thread there are 3 perplexed opinions from three
different committers, and nobody pleading in favor of this patch
except me, so let's simply mark this as rejected and move on.
Well I also don't really like the existing behavior but it's not a big
issue and so I'm OK with letting go.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers