Diesel: Importing Diesel macros (Rust 2018)

Created on 24 Jun 2018  ·  19Comments  ·  Source: diesel-rs/diesel

In the 2018 edition, #[macro_use] is going away (kinda'), as use works with macros. Unfortunately, Diesel doesn't seem to play well with that, as a lot of macros are hard to find (e.g. no_arg_sql_function_body), or seem not to work even after being imported (although it's quite possible that I used a wrong path).

https://www.reddit.com/r/rust/comments/8tbah2/my_problems_with_understanding_rust_edition_2018/e166pdp/ suggests a simple work-around, but that might require a minimum compiler version bump.

#[macro_use] is probably still going to be supported, but it might be worth fixing this.

Related: https://github.com/rust-lang/rust/issues/35896#issuecomment-394553992.

Most helpful comment

@kamirr It's my understanding that the contributors' stance on issues like this is that, if there is an "easy enough" resolution, it should not be prioritized. This is similar with deprecation warnings with the Queryable derive macro. In my opinion, this does make the project feel less idiomatic and "hacky" compared to e.g. serde where you can just use serde::{Deserialize, Serialize}; in the modern way. This crate will come with the caveats of "don't forget to set this crate attribute and import all of its macros, unlike all of your other dependencies", which may not be a hard thing to accommodate as a user, but will negatively impact the image of the crate's polish and maintenance.

I personally don't care to discuss these issues if the maintainers don't care about it, but there are only so many times a project can brush off these requests before it builds up to a serious issue.

All 19 comments

I don't think there's anything actionable that we can do at this point in time. Even the comment you've linked to says "the lang team is looking into this but hasn't made a decision yet". As far as I can tell there's no compiler that demonstrates a concrete problem here, nor is there any change we can currently make that would address this potential problem today.

In a few months, if Diesel is unable to compile with stable Rust (which would be a pretty shocking step for Rust to take), please open a new issue.

As far as I can tell there's no compiler that demonstrates a concrete problem here

The issue can be observed on nightly, with the use_extern_macros feature enabled: https://play.rust-lang.org/?gist=51baef8107264c2aafaf140c4d045e86&version=nightly&mode=debug, and it affects Diesel in the same way.

I'll post here again if use_extern_macros gets stabilized and it's still hard to use it with Diesel.

I'm using edition = "2018" which will be hitting Stable Rust in 2 days (6 December, 2018). The current version of diesel_cli prints a schema.rs file without the necessary macros used, meaning I have to add it manually on each migration.

$ diesel --version
diesel 1.3.1

I think this merits fixing, as builds will start failing once people start migrating to Edition 2018. Thank you.

Using #[macro_use] extern crate diesel in your crate root continues to work on the 2018 edition, so there is no need to fix this now.
Otherwise there is also the possibility to apply a patch file to the generated schema that allows you to automate such changes.

If someone is willing to work on this feel free to submit a PR, but I've currently other priorities than "fixing" something that's already working.

If someone is willing to work on this feel free to submit a PR, but I've currently other priorities than "fixing" something that's already working.

Using macros from parent modules now results in a warning and will cause a hard error in the future, so you can't really call using #[macro_use] extern crate diesel a solution.

Also, I attempted to patch the schema file as you suggested, but I failed to get it working. I'd be grateful if you told me what exactly should I put in the schema.rs to get it to compile.

@kamirr It's my understanding that the contributors' stance on issues like this is that, if there is an "easy enough" resolution, it should not be prioritized. This is similar with deprecation warnings with the Queryable derive macro. In my opinion, this does make the project feel less idiomatic and "hacky" compared to e.g. serde where you can just use serde::{Deserialize, Serialize}; in the modern way. This crate will come with the caveats of "don't forget to set this crate attribute and import all of its macros, unlike all of your other dependencies", which may not be a hard thing to accommodate as a user, but will negatively impact the image of the crate's polish and maintenance.

I personally don't care to discuss these issues if the maintainers don't care about it, but there are only so many times a project can brush off these requests before it builds up to a serious issue.

@rakenodiax @kamirr AFAIK all of the diesel warnings are fixed in master and will at some point be found in a future release. If you want a solution now, please use the diesel master branch.

@technetos Seems like a good idea. There's this tiny problem that the master branch doesn't build ATM, but I guess I can wait.

I think those last comments mix some different things.

  1. Those warnings issued on the latest release if a derive is used.
  2. Making diesel compatible with importing macros by use statements.

The first point is unrelated to this issue and already fixed on master. Master currently builds fine, but our ci fails because some dependency bumped its minimal supported rust version above ours. So if you don't need to support rust 1.27 or something you could just use master.

The second point is somewhat more complex. I think we all agree that this should be fixed, but it's unfortunately quite complex to do this. I've already a branch somewhere that works in principal. The issue is that rustdoc does not allow me to document those macros in a way I would like. I need to find some time to look into how we could workaround this.

The standard solution of how to use 2018 macros and be backwards compatible is in the edition guide: macro changes. Someone could implement this for diesel now. The bit I wouldn't know how to do is how to generate the table! macro in schema.rs differently in the different editions. Maybe there could be an option on diesel_cli for the edition, and it tries to detect it from Cargo.toml otherwise.

@derekdreery There is already #1956. The only thing missing there is putting all macros exports in the right place.

This issue seems to have been closed a while ago, has it been fixed or just marked as wont fix ?

@weiznich , you have closed my issue #2386 with the reference to this issue. My apologies for opening a duplicate. This issue seems to be closed, however, there is still no way (at least in the docs) to use diesel without #[macro_use]. Should this be reopened, or am I missing something in the docs/examples?

@kdubovikov Please take the time to read to whole thread. The latest released diesel version just does not support importing macros without #[macro_use] as it's not just a documentation issue, but needs quite a lot of support from the crate offering the macros.

@kdubovikov Please take the time to read to whole thread. The latest released diesel version just does not support importing macros without #[macro_use] as it's not just a documentation issue, but needs quite a lot of support from the crate offering the macros.

@weiznich Thanks, I have read it before raising the question. I understand that this is not a documentation issue. The problem is that this ticket was closed without mentioning the final decision:

  1. Diesel will support this and it will be implemented. If this is true, I am not sure why this issue should be closed. Is there any other open issue that adds this on the roadmap?
  2. Diesel will not support this and this issue is closed. Nothing will be done further, at least for the current time

@mitermayer also asks the same question.

The last comment I see is:

@derekdreery There is already #1956. The only thing missing there is putting all macros exports in the right place.

1956 is closed and merged. I have also found #2248, which seems to be closely related to this issue. This pull request is also merged into the master branch. Does this mean that diesel already supports new-style macro imports, but this feature is not yet included in current stable release?

Thanks a lot for being supportive and answering on all questions in a very timely manner 👍🏻

@kdubovikov As you already figured out from the linked PR's it is already implemented on master, so there is no need to discuss anything else here. Additionally you can see there was no new major release since those PR's where merged, therefore no current diesel release supports this. (Please don't ask for a timeline for the next major release…)

Beside of that: Our issue tracker is a place for us developers to track thinks we are working on, not a place to answer support questions or anything like that. (Use our gitter channel or forum for that). A open issue here indicts that this is something someone could start working on, a closed issue is something that cannot be worked on for whatever reason.

It would be really beneficial if the recommended documentation such as http://diesel.rs/guides/getting-started/ didn't lead you astray. The examples that are in the source tree (at the same tag) are helpful, but it's not obvious that the getting started guide will lead you to errors that are one or two steps removed from the origin source (e.g. "cannot find __diesel_parse_table!")

@jonringer I don't know why you feel that the getting started guide leads you astray, as it has a #[macro_use] extern crate diesel; in literally the first rust code block on that page. Anyway if you have some concrete suggestions for improvements: You will find the source code here, just open a PR with your improvements there.

in literally the first rust code block on that page

I must have started from another post, then stumbled upon the getting started later and was selectively trying to "patch" my code to work. You're correct though, following the guide strictly does work. I apologize.

just open a PR with your improvements there.

absolutely.

Sorry for the noise.

Was this page helpful?
0 / 5 - 0 ratings