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.

Sign up for GitHub

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

Jump to bottom

F/afsql warning fixes #801

Closed
wants to merge 3 commits into from
Closed

F/afsql warning fixes #801

wants to merge 3 commits into from

Conversation

bazsi
Copy link
Collaborator

@bazsi bazsi commented Nov 24, 2015

This one resolves #776 by fixing warnings in the afsql module. It bumps the libdbi requirement to
0.9.0 but that should already be available on all major distributions.

@bazsi
afsql: remove remainders of ENABLE_SQL condition compilation
043f789 
The entire module is either compiled or not in case the required
dependencies are available, no need to support conditional compilations
within the module. Parts of that have recently been removed, but the
grammar still had one instance, remove that too.

Signed-off-by: Balazs Scheidler <balazs.scheidler@balabit.com>
@bazsi
afsql: bump minimal version of libdbi to 0.9.0 and require newer inte…
8880303 
…rfaces

Signed-off-by: Balazs Scheidler <balazs.scheidler@balabit.com>
@bazsi
Copy link
Collaborator Author

bazsi commented Nov 26, 2015

hmm... travis seems to run on precise where only 0.8.x is available, which does not compile with this change. :(

@ihrwein
Copy link
Contributor

ihrwein commented Nov 26, 2015

Shouldn't the configure script disable the sql module with your change?

@bazsi
Copy link
Collaborator Author

bazsi commented Nov 27, 2015

Yeah it should (a bug in itself) but then we'd not test afsql in travis.

I'll check that.
On Nov 26, 2015 4:46 PM, "Tibor Benke" notifications@github.com wrote:

Shouldn't the configure script disable the sql module with your change?

—
Reply to this email directly or view it on GitHub
#801 (comment).

@bazsi
configure.ac: fix up LIBDBI check to actually drive the afsql decision
43d1b68 
Signed-off-by: Balazs Scheidler <balazs.scheidler@balabit.com>
@ihrwein
Copy link
Contributor

ihrwein commented Nov 30, 2015

👍

1 similar comment
@nvxxu2i
Copy link
Contributor

nvxxu2i commented Nov 30, 2015

👍

@bazsi
Copy link
Collaborator Author

bazsi commented Dec 7, 2015

@lbudai it seems that we don't have the libdbi dependencies in OBS yet and until that happens this PR would disable SQL tests in travis.

What should we do? I see three options:

  1. not merge this
  2. merge this now and open an issue to add libdbi to OBS
  3. wait until libdbi 0.9 is in OBS and then merge this.

What do you think?

@lbudai
Copy link
Collaborator

lbudai commented Dec 8, 2015

@bazsi: I think OBS shouldn't be a problem.
(I can release the libdbi 0.9.x in OBS, like other deps, so I'd merge this now.)

@lbudai lbudai mentioned this pull request Dec 8, 2015
Merged
@ihrwein
Copy link
Contributor

ihrwein commented Dec 8, 2015

The rebased version of this PR (#829) has been merged. I'm closing this.

@ihrwein ihrwein closed this Dec 8, 2015
@bazsi bazsi deleted the f/afsql-warning-fixes branch January 14, 2016 06:51
@bazsi bazsi restored the f/afsql-warning-fixes branch January 14, 2016 06:51
@bazsi bazsi deleted the f/afsql-warning-fixes branch January 14, 2016 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Eliminate compiler warnings and turn on -Werr
4 participants
@bazsi @ihrwein @nvxxu2i @lbudai