Added a VERGEN_GIT_DIRTY output to git2 and gitcl

[hgomersall]

Currently no GIT_DIRTY output is provided for gix as it wasn't obvious how to use the API to do that.

[hgomersall]

Resolves #237

[hgomersall]

There's a mismatch in clippy lints - I expect from different versions of rustc being used. I'll push a fix and see what happens. What version of rustc is used by the CI pipeline?

[hgomersall]

Just noticed it's in the check description!

[hgomersall]

A couple of lint errors were firing on my machine (rustc 1.74) which aren't showing up on any of the CI runs which confuses me, since I'd expect them on nightly or beta; also the lint error that is showing up on the CI run is not showing up on my machine (probably a feature mismatch). In any case, there is a fix for both in the latest commit.

[CraZySacX]

If the lints fail again I'll take a look. The CI actually runs against the MSRV (1.70 right now), as well as nightly, beta, and stable.

Looks like formatting failed. A quick cargo fmt should be all that's needed.

[hgomersall]

Ach, I forgot to run fmt on the result from clippy. I've just pushed an update with the single comma removed that upset the formatting check.

[CraZySacX]

Ah yeah, they are all failing for the same reason. Just flip the if/else condition and that lint will be satisfied:

 error: unnecessary boolean `not` operation
   --> vergen/src/feature/git/cmd.rs:650:17
    |
650 | /                 if !output.stdout.is_empty() {
651 | |                     add_map_entry(VergenKey::GitDirty, "true", map);
652 | |                 } else {
653 | |                     add_map_entry(VergenKey::GitDirty, "false", map);
654 | |                 }
    | |_________________^
    |
    = help: remove the `!` and swap the blocks of the `if`/`else`

[hgomersall]

Fixed I hope. Is there a something that should be done about gix?

[CraZySacX]

For reference, there is a script at the top level ./run_all.fish that I use locally to run the same commands as the CI system. You could reference that for the clippy commands. I really need to update the CONTRIBUTING doc, looks out of date.

[hgomersall]

run_all.fish is giving me a load of issues with needless borrows for generic args:

error: the borrowed expression implements the required traits
   --> vergen/tests/git_output.rs:181:43
    |
181 |             let _res = fs::remove_dir_all(&path);
    |                                           ^^^^^ help: change this to: `path`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args
    = note: `-D clippy::needless-borrows-for-generic-args` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::needless_borrows_for_generic_args)]`

[CraZySacX]

Regarding gix, I'll do some research when this gets merged. As I recall, there is a way to check dirty status. If there isn't something obvious, I'll reach out to the maintainer, he's usually pretty good about responding. Btw, thanks for putting in this PR, I know the lints are painful, but helps keep everything consistent.

[hgomersall]

Oh, they're mine! I'll fix them!

[hgomersall]

I think those lints should be fixed. The remaining ones are issues with gix (actual compilation errors). I haven't removed them because it will muddy things once the gix stuff is added. I'm happy to fold those changes in (which should be pretty trivial) if you can advise how to do it.

[hgomersall]

This should probably be considered a [WIP] until then.

[hgomersall]

I think those lints should be fixed. The remaining ones are issues with gix (actual compilation errors). I haven't removed them because it will muddy things once the gix stuff is added. I'm happy to fold those changes in (which should be pretty trivial) if you can advise how to do it.

Just to clarify - the problems now are that some of the config gates have not been removed for gix, which means for example, the constants are not defined. Removing them is possible, but then one would just have to go through an undo all those changes.

[CraZySacX]

Yep, makes sense. I'll dig into gix and see what I can find out.

[CraZySacX]

I think I have a path forward with gix, however it won't support git_dirty_ignore_untracked. Once gix has finished the implementation of the gix_status crate, I'll update it. I'm working on fixing the git_output tests at the moment. I like what you did with the TestRepo struct. However, the repo that was being created was already dirty from a tag reference by default, so I'm working through adjusting it to only be dirty when specified.

[hgomersall]

I think I have a path forward with gix, however it won't support git_dirty_ignore_untracked. Once gix has finished the implementation of the gix_status crate, I'll update it. I'm working on fixing the git_output tests at the moment. I like what you did with the TestRepo struct. However, the repo that was being created was already dirty from a tag reference by default, so I'm working through adjusting it to only be dirty when specified.

The git_dirty_unset test in the git_output.rs tests should fail if the repo is initialized dirty. Is this a bug in the implementation?

How shall we handle the ignore untracked stuff on gix? We could gate the implementation to ignore or disallow that flag for gix?

[CraZySacX]

I think I have a path forward with gix, however it won't support git_dirty_ignore_untracked. Once gix has finished the implementation of the gix_status crate, I'll update it. I'm working on fixing the git_output tests at the moment. I like what you did with the TestRepo struct. However, the repo that was being created was already dirty from a tag reference by default, so I'm working through adjusting it to only be dirty when specified.

The git_dirty_unset test in the git_output.rs tests should fail if the repo is initialized dirty. Is this a bug in the implementation?

How shall we handle the ignore untracked stuff on gix? We could gate the implementation to ignore or disallow that flag for gix?

gix at this point can only tell if there are changes in the git index which is half of dirty (the other half being untracked unless ignored). The gix developer is currently working on git gix-dir package which should help with the untracked files portion of dirty. For now, I'm going to leave the partial implementation in, with a warning in the documentation that the feature isn't complete.

Regarding the tests, the setup is correct, but my initial implementation of the dirty check for gix was wrong, so the failures were correct.

[hgomersall]

Do you need any input from me for the time being?

[hgomersall]

I like what you did with the TestRepo struct.

I toyed with diverging more from the general structure but I didn't want to commit a load of effort if it was not acceptable or broke other tests. If I did it from scratch I think I'd bring the directory naming inside the struct and use a random temp directory to avoid the issue of clean-up failure breaking subsequent runs. It should be generally extendable to directly test the output for a given emitted output over and above a regex.

[CraZySacX]

I like what you did with the TestRepo struct.

I toyed with diverging more from the general structure but I didn't want to commit a load of effort if it was not acceptable or broke other tests. If I did it from scratch I think I'd bring the directory naming inside the struct and use a random temp directory to avoid the issue of clean-up failure breaking subsequent runs. It should be generally extendable to directly test the output for a given emitted output over and above a regex.

I did pull TestRepo out into a module and did make the bare repo and clone directories name contain 5 random chars. Regarding the gix implementation, I cannot fully implement dirty without jumping through some more complex hoops than I want to. I'm going to leave the flag in, but note that it will always generate false if using the gitoxide feature until gix has a more clean way to emulate git status. There is currently index_as_worktree but it appears it's isn't 100% reliable yet per comments in this PR

[CraZySacX]

This code has been merged from #283