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

fix: actually use sqlite-dynlib feature #504

Merged
merged 1 commit into from
Oct 30, 2022

Conversation

jcgruenhage
Copy link
Contributor

Follow-up on #474

7d721a1 introduced the sqlite-dynlib feature, but using it in places
where sqlite is currently used was not possible, because the cfg gates
weren't adjusted yet. This commit replaces each feature = "sqlite"
with any(feature = "sqlite",feature = "sqlite-dynlib"), meaning that
it doesn't matter which sqlite feature you use, reedline will expose the
same API.

7d721a1 introduced the sqlite-dynlib feature, but using it in places
where sqlite is currently used was not possible, because the cfg gates
weren't adjusted yet. This commit replaces each `feature = "sqlite"`
with `any(feature = "sqlite",feature = "sqlite-dynlib")`, meaning that
it doesn't matter which sqlite feature you use, reedline will expose the
same API.
@sholderbach
Copy link
Member

Right I totally forgot the impact in the code. Sad that the dynamic linking is a bit hard to test with the default actions runner.

Thanks for the fix!

@sholderbach sholderbach merged commit 698f4eb into nushell:main Oct 30, 2022
@jcgruenhage
Copy link
Contributor Author

No worries, I also didn't think about it back then, just stumbled over it now, when trying to build our patched nushell package:

error[E0432]: unresolved import `reedline::SqliteBackedHistory`
  --> crates/nu-cli/src/repl.rs:21:51
   |
21 | use reedline::{DefaultHinter, EditCommand, Emacs, SqliteBackedHistory, Vi};
   |                                                   ^^^^^^^^^^^^^^^^^^^
   |                                                   |
   |                                                   no `SqliteBackedHistory` in the root
   |                                                   help: a similar name exists in the module: `FileBackedHistory`

error[E0432]: unresolved import `reedline::SqliteBackedHistory`
 --> crates/nu-command/src/misc/history.rs:9:5
  |
9 |     SqliteBackedHistory,
  |     ^^^^^^^^^^^^^^^^^^^
  |     |
  |     no `SqliteBackedHistory` in the root
  |     help: a similar name exists in the module: `FileBackedHistory`

It looks to me that this works as expected now, although I haven't quite finished up the packaging of 0.70.0 for void yet

@jcgruenhage jcgruenhage deleted the sqlite-dynlib-usage branch October 30, 2022 15:22
@sholderbach
Copy link
Member

Thank you for putting in the hard work to deal with the packaging! Great to have nushell available in more distributions.

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.

2 participants