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

Diesel Migration Panics #2802

Closed
marvin-hansen opened this issue Aug 20, 2024 · 2 comments
Closed

Diesel Migration Panics #2802

marvin-hansen opened this issue Aug 20, 2024 · 2 comments

Comments

@marvin-hansen
Copy link
Contributor

Bazel: 7.3.0
rules_rust: 0.49.1
rust: 1.80.1

I have a crate that uses the diesel and the diesel_migrations crate to embed .sql files,
but no matter what I try, the macro panics during build and reports t the sql files not being found.

The error:

16 | pub const MIGRATIONS: EmbeddedMigrations = embed_migrations!("migrations");
   |                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   = help: message: Failed to receive migrations dir from Some("migrations")
error: aborting due to 1 previous error

The same crate builds fine with Cargo.

Full reproduction of the issue: https://github.com/marvin-hansen/bazel-diesel-postgres

Note, the crate was recently donated as code example to the Diesel project, so just ignore the fluff about custom Postgres types.

I did some research and found a few hints:

Expanding on the last point, the macro constructs the path to the migration folder as following:

    let cargo_toml_directory = env::var("CARGO_MANIFEST_DIR")?;
    let cargo_manifest_path = Path::new(&cargo_toml_directory);
    let migrations_path = given_path.as_ref().map(Path::new);

    resolve_migrations_directory(cargo_manifest_path, migrations_path)

fn resolve_migrations_directory(
    cargo_manifest_dir: &Path,
    relative_path_to_migrations: Option<&Path>,
) -> Result<PathBuf, Box<dyn Error + Send + Sync>> {
    let result = match relative_path_to_migrations {
        Some(dir) => cargo_manifest_dir.join(dir),
        None => {
    ....
        }
    };

Here, the CARGO_MANIFEST_DIR is set, I have verified that.
The migrations_path can be set either in the Diesel.toml config or as an optional argument to the macro.
I did both, defined in the Diesel config file and set the same folder name as an argument to the macro.
And then the resolve_migrations_directorym that to throw the error, joins the two together as the path to find the migration folder.

In the repro, the final relative path should be something like:

/bazel-diesel-postgres/migrations

And both, the folder and the Diesel.toml config are copied into the Bazel sandbox.

And yet, somehow the macro keeps panicking.

@marvin-hansen
Copy link
Contributor Author

marvin-hansen commented Aug 20, 2024

Seems like this can be solved by overwriting the CARGO_MANIFEST_DIR environment variable
and declaring the migration folder as compile_data dependencies .

It is unclear to me why I have to overwrite CARGO_MANIFEST_DIR but it's definitely needed otherwise the macro panics.

The config below works, though.

load("@bazel_skylib//rules:copy_directory.bzl", "copy_directory")
load("@bazel_skylib//rules:copy_file.bzl", "copy_file")
load("@rules_rust//rust:defs.bzl", "rust_doc", "rust_doc_test", "rust_library")

filegroup(
    name = "diesel_toml",
    srcs = glob([
        "diesel.toml",
    ]),
)

copy_directory(
    name = "copy_migrations",
    src = "migrations",
    out = "migrations",
)

rust_library(
    name = "services",
    srcs = glob([ "src/**"]),
    crate_root = "src/lib.rs",
    compile_data = [
       ":diesel_toml",
       ":copy_migrations",
    ],
    rustc_env = {
        "CARGO_MANIFEST_DIR": "/absolute/path/to/bazel-diesel-postgres",
    },
    deps = [
        # External crates
        "@crates//:diesel",
        "@crates//:diesel_migrations",
    ],
    visibility = ["//visibility:public"],
)

@marvin-hansen
Copy link
Contributor Author

Closing this because the CARGO_MANIFEST_DIR env variable can be generated with a genfile.
While it is unclear if this is a feature or a bug, it's not worth fixing IMHO.

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

No branches or pull requests

1 participant