Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow build-from-source on GNU/Linux without maven repositories #546

Merged
merged 8 commits into from
May 10, 2016

Conversation

praiskup
Copy link
Member

No description provided.

Pavel Kajaba and others added 3 commits April 12, 2016 15:09
This is needed for GNU/Linux systems at the moment where
waffle-jna.jar is not usually available in a native package
(built-from-source on GNU/Linux).
This commit is aimed to ease the building of pgjdbc from source on
GNU/Linux hosts "offline";  without having  some of the
dependencies installed (otherwise strictly required) and without
some features built-in.

There are two new properties, waffleEnabled and osgiEnabled, both
are set to 'true' by default and are used to turn on/off
corresponding maven profiles.

By specifying '-DwaffleEnabled=false' during maven build we can
build pgjdbc.jar without Windows SSPI authentication support.
By specifying '-DosgiEnabled=false' respectively, you can build
pgjdbc.jar without OSGi Enterprise support.
@codecov-io
Copy link

Current coverage is 58.07%

Merging #546 into master will decrease coverage by -0.01% as of f18c9b1

@@            master    #546   diff @@
======================================
  Files          147     147       
  Stmts        15492   15498     +6
  Branches      3059    3059       
  Methods          0       0       
======================================
+ Hit           8998    9001     +3
+ Partial       1211    1209     -2
- Missed        5283    5288     +5

Review entire Coverage Diff as of f18c9b1

Powered by Codecov. Updated on successful CI builds.

@pkajaba
Copy link

pkajaba commented Apr 14, 2016

Any thoughts about this? @vlsi

@vlsi
Copy link
Member

vlsi commented Apr 14, 2016

@pkajaba , thank you. The change looks OK in general, however:

  1. There should be Travis job that performs build with those new options (to validate that build succeeds).
  2. Naming public class ISSPIClient { is wrong. One does not use I prefix for a class name.

I'm sorry my schedule is tight, so I cannot merge the change right away.

@pkajaba
Copy link

pkajaba commented Apr 14, 2016

No problem, take your time. We fix these comments :)

I meant with that I at the beginning of name of the class to indicate some kind of interface.

@vlsi
Copy link
Member

vlsi commented Apr 15, 2016

@pkajaba , adding an item to .travis.yml and fixing method naming is not a big deal. I can do that as merge the change.

What I am much less aware is the way to CI-test via xmvn or whatever way you are going to use.
Can you please check/findout the way xmvn CI can be triggered from Travis?

@praiskup
Copy link
Member Author

@vlsi , do we need to test xmvn directly, or is it enough to tetst 'mvn' command with
those options enabled? As far as I've heard, travis CI does not work for Fedora --
and 'xmvn' is, based on @pkajaba, yet to be packaged for Debian/Ubuntu.

It should not be a big issue anyway, by enabling the options 'mvn' should produce the same
results as 'xmvn'.

Also, we can think about using Copr as CI (discussed before) - but I'm not sure copr
can be hooked here into github.

@davecramer
Copy link
Member

@praiskup I think @vlsi is thinking that travis-ci could trigger a build on your system. You have a full ubuntu machine at your disposal, you could either ssh to somewhere else or whatever ...

@vlsi
Copy link
Member

vlsi commented Apr 19, 2016

Also, we can think about using Copr as CI (discussed before) - but I'm not sure copr
can be hooked here into github.

That is what I mean.
Travis CI can hopefully issue HTTP(s) request to trigger Copr validation & wait.

@praiskup
Copy link
Member Author

I've checked the possibilities -- and as far as I understand, we can't simply trigger the build by webhook, this option is not implemented in Copr that way to be compatible with pgjdbc conventions (there are Mock SCM and Tito options, but both are not suitable for pgjdbc). And both would have complicated checking for build results to "block PR" in case of failure.

So the option to me is to (a) generate SRPM on ubuntu and (b) make it available somewhere and then (c) submit a build into Copr (that might be implemented by raw HTTP request) but not completely trivial. The (a) point I can't say how trivial is as I dont know how easy it is generate SRPM on ubuntu, and the (c) is there because we probably can't simply install copr client on ubuntu.

Is it worth the troubles now ^^? To me, the more suitable option would be to implement new build-option into Copr (something which takes (a) spec file and (b) links to tarballs, I can work on such patch against Copr). This would require that we are able to obtain both tarballs -- pgjdbc-parent-poms and pgjdbc from PRs. Once this would be deployed, we could add the CI here into
github. In the meantime we can keep an eye here manually. What do you think?

@praiskup
Copy link
Member Author

Of course, we can build the jars with 'mvn -PwaffleEnabled=false' directly, and check that build/test
passes?

@vlsi
Copy link
Member

vlsi commented Apr 20, 2016

Does https://developer.fedoraproject.org/deployment/copr/copr-cli.html work for triggering a copr build?

@praiskup
Copy link
Member Author

praiskup commented Apr 20, 2016

Yes. But you need to have SRPM first and have the copr cli available.
So we usually do "rpmbuild -bs *.spec" or something similar on Fedora, then we can
do 'copr build PROJECT_ID your.src.rpm'.

@vlsi
Copy link
Member

vlsi commented Apr 20, 2016

That's unfortunate. Ok, let's stick with mvn -PwaffleEnabled=false kind of Travis job.

@praiskup
Copy link
Member Author

I've added travis job with disabled osgi/waffle, but I'm afraid that the dependencies are
still downloaded, because this PR does not reference the pull request pgjdbc/pgjdbc-parent-poms#4
So full testing will be possible once that PR is merged.

@praiskup
Copy link
Member Author

Ah, thanks for the pgjdbc/pgjdbc-parent-poms#4 merge! Ok, I've fixed the travis.yml
a bit and it seems to test what we need -- well at least those waffle/osgi related files
are not built, osgi is not tested, but I'm not 100% sure what is happening there.

Is there something we could help with now (before we hack Copr to be able to serve
as CI for pgjdbc)?

@vlsi
Copy link
Member

vlsi commented Apr 21, 2016

a bit and it seems to test what we need -- well at least those waffle/osgi related files

I'm afraid it does not.
In order to leverage new parent-poms, you need to update version of parent-poms in dependency (somewhere around here: https://github.com/pgjdbc/pgjdbc/blob/master/pom.xml#L6)

The v1.0.6 has Waffle/OSGi dependencies optional.
This is unfortunate because users who want to build without OSGIi
and without Waffle need to specify yet another compilation flag.

But there is no way to do this automatically when user specifies
'-DwaffleEnabled', the reason is that excludePackageNames accepts
one string (colon or comma separated list of package names), and
we can not edit the excludePackageNames tag twice (firstly for
waffleEnabled and then for osgiEnabled properties) -- the last
excludePackageNames wins.
@praiskup
Copy link
Member Author

In order to leverage new parent-poms, you need to update version of parent-poms in dependency
(somewhere around here: https://github.com/pgjdbc/pgjdbc/blob/master/pom.xml#L6)

You were right - when pgjdbc-parent-poms started handling the new options, maven build
failed later on javadoc generation. There is yet another patch in this PR to fix that.

@praiskup
Copy link
Member Author

praiskup commented May 9, 2016

Ping :)

@davecramer davecramer merged commit 87489a9 into pgjdbc:master May 10, 2016
return (ISSPIClient) c.getDeclaredConstructor(cArg).newInstance(pgStream, spnServiceClass, enableNegotiate, logger);
} catch (Exception e) {
// This catched quite a lot exceptions, but until Java 7 there is no ReflectiveOperationException
throw new UnsupportedOperationException("You are using jar from Linux distribution or class SPPIClient cannot be loaded");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was not good. Original exception was not included into the new one, so user would have no way to debug the failure.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vlsi, right - thanks for the fix!

@praiskup
Copy link
Member Author

Thanks for the merge, perfect! I can't wait for the next release :).

@vlsi
Copy link
Member

vlsi commented May 10, 2016

@praiskup , does your packaging work as expected? Do "package consumers" confirm the package works for them?

@praiskup
Copy link
Member Author

@vlsi, I'll give it a concrete try -- but as long as the travis build with disabled features
reports success, there shouldn't be issue.

@praiskup
Copy link
Member Author

@vlsi, what way I can generate the release tarball you provide during releases? 'mvn package'
has different (not so good) structure for RPM package rebuild.

@vlsi
Copy link
Member

vlsi commented May 10, 2016

"In theory there is no difference between theory and practice, in practice there is...".

You mean "dist.tar.gz" file?
It is produced by mvn package -P release-artifacts.

@praiskup
Copy link
Member Author

On Tuesday 10 of May 2016 01:40:17 Vladimir Sitnikov wrote:

"In theory there is no difference between theory and practice, in practice there is...".

:) I bet I should be more concrete. Well, what is scary about the
pgjdbc.tar.gz obtained by 'mvn package' only is that it is a tarbomb --
trust me, people in unix world hate that.

Otherwise -- if the file hierarchy within tarball changes a bit,
downstream packagers need to fix theirs scripts (which is not a problem
per se if it does not happen too often) and that is what motivates me to
have the postgresql-9.4.1209-SNAPSHOT-dist.tar.gz in hand.

You mean "dist.tar.gz" file?
It is produced by mvn package -P release-artifacts.

This command helped, thanks!

@vlsi
Copy link
Member

vlsi commented May 10, 2016

trust me, people in unix world hate that

Any idea what should be better there?
Frankly speaking, I've no idea who uses that dist artifact.

@praiskup
Copy link
Member Author

praiskup commented May 10, 2016

@vlsi wrote: Any idea what should be better there?

Ideally (not saying we must have it) there should be one tarball pattern
like PROJECT-VERSION.tar.gz, that should contain one top-level
directory named PROJECT-VERSION and everything else would be underneath.

There shouldn't be any pre-compiled stuff (usually also documentation
sources, not documentation - if any exists).

I'm not sure how to fix this; even the '*-dist.tar.gz' tarball is not
100% perfect but is much better than the pgjdbc.tar.gz tarball.

@vlsi wrote: Frankly speaking, I've no idea who uses that dist artifact.

All GNU/Linux package maintainers, I believe.

@praiskup
Copy link
Member Author

praiskup commented May 31, 2016

@vlsi, I'm trying to have a look at the CI part of this topic. The problem is that in Copr,
it is not easy to produce the "right" tarball artifact. On Ubuntu (or Debian?) that is running
in travis is not that easy to install copr client and maybe rpm packaging tools.

So, on the Ubuntu machine, would that be possible to run containerized Fedora? This is
probably not 100% perfect solution (running Ubuntu kernel against Fedora container), but
it might work for us like on ubuntu machine:

  $ git clone ...
  $ mvn -P release-artifacts ...
  $ mkdir -p bind-mount
  $ cp tarball copr-credentials bind-mount
  $ docker run -ti FEDORA_IMAGE -v `pwd`/bind-mount:/sources 
  ....
  ....
  $ echo $?
  0

Would that be problem to execute docker image based on Fedora?

The other option is that we can provide long-term runnning virtual machine
with pre-installed copr client, credentials, etc. The VM running in Travis
would just ssh to the Fedora box and it could spawn the copr build from there.

@vlsi
Copy link
Member

vlsi commented May 31, 2016

Travis supports docker: https://docs.travis-ci.com/user/docker/, so running against Fedora container should be possible.

praiskup added a commit to devexp-db/pkg-postgresql-jdbc-maven-move that referenced this pull request May 31, 2016
See this link for more info:
pgjdbc/pgjdbc#546

Version: 9.4.1208-8
@praiskup
Copy link
Member Author

praiskup commented Jun 1, 2016

FTR, continuing in #578

@vlsi vlsi added this to the 9.4.1209 milestone Jul 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants