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

[SQLite] Travis is failing for... reasons... #167

Closed
sgrif opened this Issue Feb 1, 2016 · 11 comments

Comments

Projects
None yet
2 participants
@sgrif
Member

sgrif commented Feb 1, 2016

Note: PRs addressing this issue should target the diesel-sqlite-support branch, not master.

This has been absurdly painful to debug. We need to fix it before we can merge into master, but I really am at my wits end about this. We've had to change our build infrastructure, because travis doesn't have a "not fucking ancient" option for the SQLite version. By default they ship a version from 2011, and we rely on sqlite3_errstr, which was added in 2012.

The new infrastructure should be using a more recent version, but for some reason we fail to compile when we get to codgen. There is absolutely no reason I can think of that this would fail at codegen, but the main crate would continue to compile.

We need to figure out some way to get the build green, either by getting travis working with the code as is, or by removing our dependency on sqlite3_errstr and switching back to the container infrastructure.

@sgrif sgrif added this to the 0.5 milestone Feb 1, 2016

@sgrif

This comment has been minimized.

Member

sgrif commented Feb 1, 2016

Oh also, somehow nightly on PG passes even though the failure is at compile time not runtime. Because... yeah... reasons...

@emptyflash

This comment has been minimized.

Contributor

emptyflash commented Feb 1, 2016

Not sure if you came across this in your searching, but apparently Travis CI provided a workaround for upgrading sqlite3, but unfortunately it only seems to work on their legacy infrastructure.

@emptyflash

This comment has been minimized.

Contributor

emptyflash commented Feb 1, 2016

Actually, scratch that. I was able to get the build to pass by manually installing the workaround and setting the distro to precise instead of trusty.

@sgrif

This comment has been minimized.

Member

sgrif commented Feb 1, 2016

I'd prefer to avoid using apt-get for this if we can. Trusty should ship
with 3.8 by default.

On Mon, Feb 1, 2016 at 11:24 AM Cameron Alexander notifications@github.com
wrote:

Actually, scratch that. I was able to get the build to pass by manually
installing the work around and setting the distro to precise instead of
trusty
https://github.com/SoftwareWarlock/diesel/blob/diesel-sqlite-support/.travis.yml#L2
.


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

@emptyflash

This comment has been minimized.

Contributor

emptyflash commented Feb 1, 2016

Agreed. Maybe Travis is manually changing the version they have installed? I can't see a reason why the build would pass from manually upgrading sqlite if the installed default version really is 3.8.

Would you be willing to consider switching to a different CI system? I've had a pretty pleasant experience using CircleCI.

@emptyflash

This comment has been minimized.

Contributor

emptyflash commented Feb 1, 2016

Just did a sanity check, and the version really is 3.8, so maybe the codegen is compatible with 3.7.15, but not 3.7.9 nor 3.8?

@sgrif

This comment has been minimized.

Member

sgrif commented Feb 1, 2016

It's definitely compatible with 3.8. If you look at the failure message
appearing on Travis, it's due to sqlite3_errstr not being found, which
means it's an ancient 3.7 version. Also note that the problem isn't that
codegen is failing to compile, but that Diesel as a dependency of codegen
is failing

On Mon, Feb 1, 2016 at 12:37 PM Cameron Alexander notifications@github.com
wrote:

Just did a sanity check, and the version really is 3.8, so maybe the
codegen is compatible with 3.7.15, but not 3.7.9 nor 3.8?


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

@sgrif

This comment has been minimized.

Member

sgrif commented Feb 1, 2016

...And I just realized what the problem is.

On Mon, Feb 1, 2016 at 12:38 PM Sean Griffin sean@seantheprogrammer.com
wrote:

It's definitely compatible with 3.8. If you look at the failure message
appearing on Travis, it's due to sqlite3_errstr not being found, which
means it's an ancient 3.7 version. Also note that the problem isn't that
codegen is failing to compile, but that Diesel as a dependency of codegen
is failing

On Mon, Feb 1, 2016 at 12:37 PM Cameron Alexander <
notifications@github.com> wrote:

Just did a sanity check, and the version really is 3.8, so maybe the
codegen is compatible with 3.7.15, but not 3.7.9 nor 3.8?


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

@sgrif

This comment has been minimized.

Member

sgrif commented Feb 1, 2016

Wait, never mind, I don't actually know what the problem is. I thought it
might have something to do with
https://github.com/sgrif/diesel/blob/master/diesel_codegen/Cargo.toml#L20,
but we override that in the cargo config to use the local path, and there's
no reason that the error it'd cause would be related to an old version of
SQLite.

On Mon, Feb 1, 2016 at 12:39 PM Sean Griffin sean@seantheprogrammer.com
wrote:

...And I just realized what the problem is.

On Mon, Feb 1, 2016 at 12:38 PM Sean Griffin sean@seantheprogrammer.com
wrote:

It's definitely compatible with 3.8. If you look at the failure message
appearing on Travis, it's due to sqlite3_errstr not being found, which
means it's an ancient 3.7 version. Also note that the problem isn't that
codegen is failing to compile, but that Diesel as a dependency of codegen
is failing

On Mon, Feb 1, 2016 at 12:37 PM Cameron Alexander <
notifications@github.com> wrote:

Just did a sanity check, and the version really is 3.8, so maybe the
codegen is compatible with 3.7.15, but not 3.7.9 nor 3.8?


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

@emptyflash

This comment has been minimized.

Contributor

emptyflash commented Feb 1, 2016

Ah, I see that now. The error I was getting original had something to do with lifetimes, but I think that was unrelated.

sgrif added a commit that referenced this issue Feb 1, 2016

Make sure Travis uses an up to date version of `sqlite`
I have no clue why this changes based on the value of `sudo: required`.
Without it, Travis uses 3.7. With it, Travis uses 3.8.

Fixes #167
@sgrif

This comment has been minimized.

Member

sgrif commented Feb 1, 2016

Fixed by a9117a3

@sgrif sgrif closed this Feb 1, 2016

sgrif added a commit that referenced this issue Feb 3, 2016

Make sure Travis uses an up to date version of `sqlite`
I have no clue why this changes based on the value of `sudo: required`.
Without it, Travis uses 3.7. With it, Travis uses 3.8.

Fixes #167

sgrif added a commit that referenced this issue Feb 3, 2016

Make sure Travis uses an up to date version of `sqlite`
I have no clue why this changes based on the value of `sudo: required`.
Without it, Travis uses 3.7. With it, Travis uses 3.8.

Fixes #167
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment