diesel-rs / diesel

A safe, extensible ORM and Query Builder for Rust
https://diesel.rs
Apache License 2.0
12.55k stars 1.05k forks source link

How to use patch_file in diesel.toml #2154

Open travismiller opened 5 years ago

travismiller commented 5 years ago

I've had a hard time working with the patch_file option with the Diesel CLI as described in the Configuring Diesel guide on the website.

Is there more practical advice for on-going usage of this feature? I feel like I'm missing something or making it harder than necessary.

As my schema has evolved, I occasionally need to make more patch revisions to the schema (so far, just Enum types) and the suggested git diff command will only output the patch from the last commit, not the entire history of patches.

I've resorted to tracking a version of the schema with no patches and re-generating the patch based on a diff of that and the current schema.

Makefile targets for convenience

# Usage:
#
# Do any modifications to `diesel.toml` and/or run migrations and then re-make the schema:
#
#    $ make src/schema.rs
#
# Do any modifications to `src/schema.rs` and then re-make the patch:
#
#    $ make src/schema.rs.patch

SHELL = /bin/bash

.PHONY: src/schema.rs
src/schema.rs:
    # patch file must exist if it's defined in diesel.toml
    touch src/schema.rs.patch
    # print the schema
    diesel print-schema > src/schema.rs
    # make unpached version
    make src/schema.rs.unpatched
    # make the new patch file
    make src/schema.rs.patch

.PHONY: src/schema.rs.patch
src/schema.rs.patch:
    diff -U6 src/schema.rs.unpatched src/schema.rs > src/schema.rs.patch || true

.PHONY: src/schema.rs.unpatched
src/schema.rs.unpatched:
    # if patch isn't empty, create schema.rs.unpatched from the existing patch
    [ ! -s src/schema.rs.patch ] || \
        patch -p0 -R -o src/schema.rs.unpatched < src/schema.rs.patch
    # otherwise create schema.rs.unpatched by copying schema.rs
    [ -f src/schema.rs.unpatched ] || \
        cp -a src/schema.rs src/schema.rs.unpatched
sgrif commented 5 years ago

Is there more practical advice for on-going usage of this feature? I feel like I'm missing something or making it harder than necessary.

You're not missing anything. This is great feedback, and a good reference for something that's been discussed internally, but I don't believe we have a tracking issue for. Patch files have definitely been a "good enough" solution to a real problem, but have very much had pain points. https://github.com/rust-lang/crates.io/commit/d9e3da67d9b5faa778a3802ab70be77864025144 and https://github.com/rust-lang/crates.io/commit/508ed318896b517b29e9bddbda0497171aa65638 are examples that I've personally run into.

There are three changes I'd like to make, two of which I think have a clear path forward for someone to take on.

  1. We should have a workflow that involves editing the schema file however you want and getting a patch file from that.

    • This is essentially the workflow mentioned in the last paragraph of https://diesel.rs/guides/configuring-diesel-cli/#the-patchfile-field, except we diff against what we think your schema should be, not what's in git. My workaround has been to delete the patch file, run diesel print-schema, commit that, apply the patch file if applicable and/or make whatever new changes I want, and then git diff that. It's... not great. We can probably also figure out the appropriate number of context lines for you, since it's most likely based on the number of items in you custom imports, and the size of the comment we generate. Changes where context lines are likely to be problematic tend to go between table invocations, so it'd be nice if we just defaulted to having "number of lines between the } of table1 and the first line where table2's name appears" and not make our users care when that changes
  2. We should maintain the patch file for you.

    • Patch files can be fickle. Your line numbers can change. A few of your context lines can change. But if it's just slightly too much, the patch no longer applies. If we're able to generate a patch file from a given schema.rs, we should just do that automatically any time a patch file successfully applied. So if one context line changed, we change that line in the patch file. If the line numbers change, we keep up to date. This leads to more churn (though churning schema.patch when schema.rs changes seems fine), but will mean fewer cases where you regenerate the file for no reason.
  3. We need a reasonable workflow for merge conflicts.

    • I have no clue where to even start here. I do not want to re-implement git in Diesel CLI. But the "edit your schema.rs and we generate a patch file" is most effective if we give you a reasonable way to recover if the patch file fails to apply.
ronaldslc commented 5 years ago

I just encountered the same issues, and for the same reasons i.e. for updating custom types dependencies. I'm wondering why not just make .sql files annotatable with similar rust like attribute syntax embedded into SQL comments.

For example:

--#sql_type="crate::models::MyCustomType"
CREATE TYPE my_custom_type AS ENUM
    ('one','two','three','four');

Which will automatically generate on any tables that uses it:

table! {
    use diesel::sql_types::*;
    use crate::models::MyCustomType;

    table1 (id) {
        id -> Uuid,
        my_enum -> MyCustomType,
    }
}

of course, this only covers a single use case, but I imagine this type of feature can be extended as more cases are discovered, then the patch file would be the last resort type of feature for cases that cannot or has not been covered.

I think it would make the out-of-the-box experience of diesel more ergonomical.

travismiller commented 4 years ago

A quick thought on this…

Is the DSL of the table! macro pretty limited? I can’t think of any customizations beyond use imports and specifying custom types for fields.

Would it be reasonable to specify the custom type overrides per field in the print_schema section of diesel.toml?

weiznich commented 4 years ago

Could you explain what exactly this would offer what is not already implemented with the import_types option?

travismiller commented 4 years ago

Sure! The existing import_types works well enough for importing anything. My pain point is specifying custom types for fields. print-schema outputs a default or generic types that I'm patching to something custom (specifically Enum in my case). Being able to explicitly specify the type in configuration could be more strait forward and reliable than the patch process.

I'm imagining adding something like this to diesel.toml:

[print_schema.tables.orders.fields]
payment_method = "Nullable<PaymentMethodMapping>"

If you wanted to use different imports per table, you could even do something like this: (but I don't think this is really necessary considering the existing import_types feature)

[print_schema.tables.orders]
import_types = [ "diesel::sql_types::*", "crate::schema_types::PaymentMethodMapping" ]

It's been a little while since I've looked at this but I'm lifting the relevant parts from my app…

Cargo.toml

[dependencies.diesel]
version = "1.4.3"
features = [ "chrono", "mysql", "numeric", "r2d2", "64-column-tables" ]

[dependencies.diesel-derive-enum]
version = "0.4.4"
features = [ "mysql" ]

diesel.toml

[print_schema]
file = "src/schema.rs"
import_types = [ "crate::schema_types::*" ]
patch_file = "src/schema.rs.patch"

# 👇 something like this might be nice… 👇

[print_schema.tables.orders]
import_types = [ "diesel::sql_types::*", "crate::schema_types::PaymentMethodMapping" ]

[print_schema.tables.orders.fields]
payment_method = "Nullable<PaymentMethodMapping>"

src/lib.rs

#[macro_use]
extern crate diesel;
#[macro_use]
extern crate diesel_derive_enum;

src/schema_types.rs

#[derive(Debug, DbEnum, Hash, Eq, PartialEq)]
pub enum PaymentMethod {
    Creditcard,
    Paypal,
    Other,
}

src/schema.rs

table! {
    use crate::schema_types::*;

    orders (id) {
        id -> Integer,
        payment_method -> Nullable<PaymentMethodMapping>, // 👈 sad trombone. This is reset to `Nullable<Enum>` during print-schema and then patched back.
    }
}

src/schema.rs.unpatched

table! {
    use crate::schema_types::*;

    orders (id) {
        id -> Integer,
        payment_method -> Nullable<Enum>,
    }
}

src/schema.rs.patch

--- src/schema.rs.unpatched 2019-08-27 16:19:42.000000000 -0500
+++ src/schema.rs   2019-08-27 16:19:42.000000000 -0500
@@ -105,15 +105,15 @@
-        payment_method -> Nullable<Enum>,
+        payment_method -> Nullable<PaymentMethodMapping>,
mox-mox commented 2 years ago

While the patch file support could definitely use some improvements, I've been inspired by @travismiller 's Makefile solution and have developed my own version. Maybe this will help someone:

# The path to the path file for the Diesel schema
SCHEMA_PATCHFILE := src/path/to/patch/file.path

# Create a new patch file
# Steps:
# 1) Make sure the variable SCHEMA_PATCHFILE is set to the same value as the variable in diesel.toml's print-schema.patch_file, e.g.
#    diesel.toml
#     ```
#     [print_schema]
#     file = "path/to/some/file"
#     patch_file = "path/to/path_file.path" <-- Must be the same as SCHEMA_PATCHFILE
#     ```
# 2) Run `make patch`
# 3) You are presented with a vim instance. The windows from left to right:
#    - First is the unmodified file generated by Diesel without any patch applied.
#    - Second is the patched file generated by Diesel. Modify this file, the generated patch file facilitates the change from the first to the second file.
#    - If a previous patch file exists, it is shown as a third file. This serves as a reminder of what changes were applied before.
# 4) Edit the second file until satisfied, then save and exit the editor. To abort exit the editor with an error by pressing ':cq'
# 5) The new patch file is generated.
patch:
    # If there is a previous patch file, copy it with a *_bak extension and store a vim command to open the patch file
    $(eval OPEN_PATCHFILE_OLD := $(shell [ ! -s $(SCHEMA_PATCHFILE) ] && read -p "No previous patch file exists. Press key to continue.. " -n1 -s || { \
        cp $(SCHEMA_PATCHFILE) $(SCHEMA_PATCHFILE)_bak ;\
        echo '-c "vsplit $(SCHEMA_PATCHFILE)" -c "setlocal readonly" -c "wincmd h"' ;}))
    # Create two temporary files. All commands after this one should succeed to ensure that the temporary files are deleted in the end
    $(eval UNPATCHED := $(shell mktemp --suffix=.rs /tmp/schema_unpatched_XXXXXX))
    $(eval PATCHED   := $(shell mktemp --suffix=.rs /tmp/schema_patched_XXXXXX))
    # Generate the unpatched schema file by overriding the patch with an empty patch file
    diesel print-schema --patch-file "" > $(UNPATCHED)
    diesel print-schema                 > $(PATCHED) || cp $(UNPATCHED) $(PATCHED)
    vim -d $(UNPATCHED) $(PATCHED) -c "setlocal readonly" -c "wincmd l" $(OPEN_PATCHFILE_OLD) && diff -U6 $(UNPATCHED) $(PATCHED) > $(SCHEMA_PATCHFILE) || true
    rm -rf $(UNPATCHED) $(PATCHED)
weiznich commented 2 years ago

@mox-mox If you see some concrete improvements, PR's are welcome.

DylanVerstraete commented 10 months ago

So, currently we have to generate a git diff file to patch a schema? What if I'm actively developing a schema, I can't imagine having to push a faulty schema to change only the related fields that need patching and generate a git diff afterwards?

weiznich commented 10 months ago

@DylanVerstraete It's highly discouraged to ask questions on stale issues. Use the support forum if you are looking for support. Otherwise: Read the issue thread, the answer to your question is already in there.