On 11/27/18 4:10 PM, Michael Paquier wrote:
> On Tue, Nov 27, 2018 at 10:27:17AM +0900, Michael Paquier wrote:
>> Okay, let's do so by supporting correctly NO_INSTALLCHECK. My other
>> refactoring work can also live with that. Returning an empty list via
>> fetchTests() and bypass a run if nothing is present looks fine to me.
>> One extra thing is that modules/commit_ts and modules/test_rls_hooks are
>> missing NO_INSTALLCHECK, so we would need to add that part to be
>> completely correct. I would also be inclined to back-patch both parts,
>> however for my stuff I could live with this patch only on HEAD, and
>> anybody willing to use installcheck on commit_ts and test_rls_hooks may
>> be surprised to not be able to do that anymore after the minor release.
>> It still looks incorrect to me though to be able to run installcheck on
>> those modules though...
>>
>> Attached is a proposal of patch, which works as I would expect with
>> modulescheck and contribcheck. How does that look?
> If possible, I would like to move on with this stuff. To keep things
> green in the buildfarm all the time, I would like to do that with two
> independent steps:
> 1) Add NO_INSTALLCHECK to modules/commit_ts and modules/test_rls_hook.
> 2) Add support for NO_INSTALLCHECK in the MSVC scripts.
>
> Are there any objections? I could do a back-patch as well to keep
> things consistent across branches if there are voices in favor of that,
> but that's not necessary for the upcoming Makefile cleanup with the new
> set of PGXS options.
I think you should just proceed with the changes above. I just had a
quick look at the patch you posted before, and it looks sane enough.
I don't see a need to backpatch.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services