Skip to content

Worktrees support in build.rs script #889

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

Closed
wants to merge 4 commits into from

Conversation

bragov4ik
Copy link
Contributor

Building the project from non-main git worktrees resulted in error "Failed to create '.git/hooks' dir". Now instead of .git in current directory the .git from the main worktree is used, making the build possible.

@bragov4ik bragov4ik force-pushed the build-worktrees-support branch from f80d3ad to c35f4a9 Compare February 6, 2024 04:34
Copy link
Contributor

@Alexey-N-Chernyshov Alexey-N-Chernyshov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting case, appreciate your effort. The bug in Jenkins clippy looks like this one. Probably Jenskins clippy uses very old rust.

@@ -52,7 +52,16 @@ fn main() {
"cargo:rerun-if-changed={}",
pre_commit_hook_path.to_string_lossy()
);
let enabled_hooks_dir = root_path.join(".git/hooks");
// Flexible path to support linked worktrees
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, add more detailed description, it is hard to catch an issue. I would add that associated worktrees share git hooks with the main worktree and thus hooks should be installed into the main one. Probably it worth to add a link to git docs.

Copy link
Contributor

@Alexey-N-Chernyshov Alexey-N-Chernyshov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also code may be simplified a bit:

    let pre_commit_hook_path = PathBuf::new()
        .join(env::var("CARGO_MANIFEST_DIR").unwrap())
        .parent()
        .unwrap()
        .join(".hooks/pre-commit");

@Alexey-N-Chernyshov
Copy link
Contributor

I checked Jenkins versions:

  • rust: 1.70-nightly, which seems to be nightly-2023-03-21
  • OS: debian bullseye 11

@bragov4ik
Copy link
Contributor Author

bragov4ik commented Feb 8, 2024

I checked Jenkins versions:

  • rust: 1.70-nightly, which seems to be nightly-2023-03-21
  • OS: debian bullseye 11

I tried to use create_dir_all in docker (image debian:bullseye) with nightly-2023-03-21-aarch64-unknown-linux-gnu toolchain and it worked well.
Code I used:

use std::path::PathBuf;
use std::{env, fs};

fn main() {
    println!("Hello, world!");
    let some_dir_1 = PathBuf::new().join("kek/");
    fs::create_dir_all(&some_dir_1).unwrap();
    fs::create_dir_all(&some_dir_1).expect("2nd create");
}

It runs well without panics.

I think it has something to do with the pipelines themselves and the way they're orchestrated/run

@bragov4ik
Copy link
Contributor Author

closing since not sure how to fix & it's almost irrelevant (nobody from the team uses git worktrees in development AFAIK)

@bragov4ik bragov4ik closed this Mar 1, 2024
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