Thread: Support waffle>1.7.4

Hello,

while lately discussion has mostly been about how to exclude waffle from
the non-Windows build, I would like to request an update in the other
direction. Last November, waffle-jna 1.7.5 was released, containing an
incompatible API change that is also present in all released 1.8
versions. The fix is simple; as far as the code goes, it is a one-line
patch. What incantations may be required on the Maven side, I do not know.

The API change was made as part of fixing a handle leak [1].

Applying this patch raises the minimum required waffle version to 1.7.5
or 1.8.0.

Thank you.

[1] <https://github.com/dblock/waffle/pull/239>

--
Christian

* Craig Ringer wrote:

> On 13 April 2016 at 01:51, Christian Ullrich <chris@chrullrich.net
> <mailto:chris@chrullrich.net>> wrote:

>     other direction. Last November, waffle-jna 1.7.5 was released,
>     containing an incompatible API change that is also present in all
>     released 1.8 versions. The fix is simple; as far as the code goes,
>     it is a one-line patch. What incantations may be required on the
>     Maven side, I do not know.
>
>     The API change was made as part of fixing a handle leak [1].
>
>     Applying this patch raises the minimum required waffle version to
>     1.7.5 or 1.8.0.
>
> Looks sensible to me, it just requires a pom.xml update to bump the
> required waffle version along with it.
>
> I'm on hols this week but will try to get it sorted out next week.
> Christian, if you send a github pull that passes tests and updates the
> pom.xml I'll just merge it directly instead.

Sorry. I'd be happy to do *that*, but it looks like that's not all there
is to it. It would be easy if the waffle version was set in pgjdbc's
pom.xml itself, but instead it references a property from the parent
(pgjdbc-versions, or possibly pgjdbc-core-parent, because apparently you
have to change it in both places before the build actually starts caring).

My current theory is that I'd have to somehow remove the property from
-core-parent (for reasons of sanity), then change it in -versions, bump
the version of -versions itself, track that change in -core-parent, put
both in the local Maven repository to fake a release, then update the
version of -versions referenced in pgjdbc so it picks them up, massage
the environment until the tests actually pass, and then send three pull
requests, against three repositories, to effectively remove 12 bytes and
change one more; three pull requests, of course, the merging of each of
which individually breaks the build. (</sentence>)

The alternative: Do nothing for a week.

I think I'll pick the second one, thank you.

--
Christian

* Craig Ringer wrote:

> On 13 April 2016 at 01:51, Christian Ullrich <chris@chrullrich.net
> <mailto:chris@chrullrich.net>> wrote:

>     other direction. Last November, waffle-jna 1.7.5 was released,
>     containing an incompatible API change that is also present in all
>     released 1.8 versions. The fix is simple; as far as the code goes,
>     it is a one-line patch. What incantations may be required on the
>     Maven side, I do not know.
>
>     The API change was made as part of fixing a handle leak [1].
>
>     Applying this patch raises the minimum required waffle version to
>     1.7.5 or 1.8.0.
>
> Looks sensible to me, it just requires a pom.xml update to bump the
> required waffle version along with it.
>
> I'm on hols this week but will try to get it sorted out next week.
> Christian, if you send a github pull that passes tests and updates the
> pom.xml I'll just merge it directly instead.

Sorry. I'd be happy to do *that*, but it looks like that's not all there
is to it. It would be easy if the waffle version was set in pgjdbc's
pom.xml itself, but instead it references a property from the parent
(pgjdbc-versions, or possibly pgjdbc-core-parent, because apparently you
have to change it in both places before the build actually starts caring).

My current theory is that I'd have to somehow remove the property from
-core-parent (for reasons of sanity), then change it in -versions, bump
the version of -versions itself, track that change in -core-parent, put
both in the local Maven repository to fake a release, then update the
version of -versions referenced in pgjdbc so it picks them up, massage
the environment until the tests actually pass, and then send three pull
requests, against three repositories, to effectively remove 12 bytes and
change one more; three pull requests, of course, the merging of each of
which individually breaks the build. (</sentence>)

The alternative: Do nothing for a week.

I think I'll pick the second one, thank you.

--
Christian

* Christian Ullrich wrote:

> * Craig Ringer wrote:

>> On 13 April 2016 at 01:51, Christian Ullrich <chris@chrullrich.net
>> <mailto:chris@chrullrich.net>> wrote:

>>     other direction. Last November, waffle-jna 1.7.5 was released,
>>     containing an incompatible API change that is also present in all
>>     released 1.8 versions. The fix is simple; as far as the code goes,

>> I'm on hols this week but will try to get it sorted out next week.
>> Christian, if you send a github pull that passes tests and updates the
>> pom.xml I'll just merge it directly instead.

> Sorry. I'd be happy to do *that*, but it looks like that's not all there
> is to it. It would be easy if the waffle version was set in pgjdbc's

I did it myself after all; I hope the result looks somewhat sensible.

- https://github.com/pgjdbc/pgjdbc-parent-poms/pull/5
- https://github.com/pgjdbc/pgjdbc/pull/555

As for "that passes tests", achieving this was not difficult (other than
getting them to work at all, see my previous PRs from today) because
there are exactly zero tests for SSPI auth, or at least that is how many
I could find. What there is, passes.

I thought about writing a few, and I may yet get around to that, but
then I collided head-on with Maven and had to sedate the Java part of my
brain.

--
Christian

* Christian Ullrich wrote:

> * Craig Ringer wrote:

>> On 13 April 2016 at 01:51, Christian Ullrich <chris@chrullrich.net
>> <mailto:chris@chrullrich.net>> wrote:

>>     other direction. Last November, waffle-jna 1.7.5 was released,
>>     containing an incompatible API change that is also present in all
>>     released 1.8 versions. The fix is simple; as far as the code goes,

>> I'm on hols this week but will try to get it sorted out next week.
>> Christian, if you send a github pull that passes tests and updates the
>> pom.xml I'll just merge it directly instead.

> Sorry. I'd be happy to do *that*, but it looks like that's not all there
> is to it. It would be easy if the waffle version was set in pgjdbc's

I did it myself after all; I hope the result looks somewhat sensible.

- https://github.com/pgjdbc/pgjdbc-parent-poms/pull/5
- https://github.com/pgjdbc/pgjdbc/pull/555

As for "that passes tests", achieving this was not difficult (other than
getting them to work at all, see my previous PRs from today) because
there are exactly zero tests for SSPI auth, or at least that is how many
I could find. What there is, passes.

I thought about writing a few, and I may yet get around to that, but
then I collided head-on with Maven and had to sedate the Java part of my
brain.

--
Christian

* Christian Ullrich wrote:

> I thought about writing a few [SSPI tests], and I may yet get around
 > to that,

Attached is a proposed patch; I cannot send it as a PR because it is
dependent on Pavel Raiskup's as yet unmerged #546. The Waffle-free build
option is clearly coming, and there is little point in having SSPI tests
that then cannot be turned off.

Some explanations:

- Both successful and unsuccessful authentication is tested, the latter
   to ensure that a configuration mistake (such as a "trust" line left
   in pg_hba.conf) has not caused *both* tests to succeed when they
   should have failed.

- Setting up to run these tests is not entirely (or at all) trivial; it
   requires running the database server as an account that is capable of
   SSPI authentication (such as a virtual service account, e.g.
   "NT SERVICE\PostgreSQL") on both domain member and non-member
   systems, or a domain user account.

- Additionally, both pg_hba.conf and, in most cases, pg_ident.conf must
   be configured. In particular, the user account that runs the tests
   must be permitted to authenticate as the database role configured in
   build.properties.

- The tests are not run when Waffle is disabled. I would have preferred
   to have a separate option to turn them off even when building with
   Waffle because the setup is so difficult. I could not think of a way
   to make Maven do this, mostly because profiles cannot be chained, and
   profile activation cannot use two variables, for example
   (!enableWaffle || disableSSPITests).

- There is an intermittent problem where testUnauthorized() fails
   because it gets the wrong exception: It expects SQLSTATE 28000 from
   the server, but sometimes it gets 08001 generated internally in the
   driver. No idea what causes that. I did not want to blindly accept any
   error as proof of failed authentication.

--
Christian

* Dave Cramer wrote:

> Ok, I just pushed pavel's PR,

Wow, that's a quick turnaround.

PR #557.

--
Christian

* Christian Ullrich wrote:

> I thought about writing a few [SSPI tests], and I may yet get around
 > to that,

Attached is a proposed patch; I cannot send it as a PR because it is
dependent on Pavel Raiskup's as yet unmerged #546. The Waffle-free build
option is clearly coming, and there is little point in having SSPI tests
that then cannot be turned off.

Some explanations:

- Both successful and unsuccessful authentication is tested, the latter
   to ensure that a configuration mistake (such as a "trust" line left
   in pg_hba.conf) has not caused *both* tests to succeed when they
   should have failed.

- Setting up to run these tests is not entirely (or at all) trivial; it
   requires running the database server as an account that is capable of
   SSPI authentication (such as a virtual service account, e.g.
   "NT SERVICE\PostgreSQL") on both domain member and non-member
   systems, or a domain user account.

- Additionally, both pg_hba.conf and, in most cases, pg_ident.conf must
   be configured. In particular, the user account that runs the tests
   must be permitted to authenticate as the database role configured in
   build.properties.

- The tests are not run when Waffle is disabled. I would have preferred
   to have a separate option to turn them off even when building with
   Waffle because the setup is so difficult. I could not think of a way
   to make Maven do this, mostly because profiles cannot be chained, and
   profile activation cannot use two variables, for example
   (!enableWaffle || disableSSPITests).

- There is an intermittent problem where testUnauthorized() fails
   because it gets the wrong exception: It expects SQLSTATE 28000 from
   the server, but sometimes it gets 08001 generated internally in the
   driver. No idea what causes that. I did not want to blindly accept any
   error as proof of failed authentication.

--
Christian

* Dave Cramer wrote:

> Ok, I just pushed pavel's PR,

Wow, that's a quick turnaround.

PR #557.

--
Christian