bazelbuild / rules_rust

Rust rules for Bazel
https://bazelbuild.github.io/rules_rust/
Apache License 2.0
667 stars 431 forks source link

Diesel Migration Panics #2802

Closed marvin-hansen closed 2 months ago

marvin-hansen commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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.