Can't pin to merge commit shas

[takeoutweight]

It seems Voom can't see merge commits. If I lein voom freshen with a merge commit as the :sha I get the error No matching version found for: deplib/deplib {:allow-snaps false, :freshen true, :repo "../deplib", :sha "thesha"}

Minimal example:

git init deplib
cd deplib
echo '(defproject deplib "0.1.0")' > project.clj
git add project.clj && git commit -m "init"
git checkout -b feature-branch
touch fileb && git add fileb && git commit -m "add fileb"
deplib_yep=`git rev-parse feature-branch`
git checkout master
git merge feature-branch --no-ff -m "merge feature-branch"
deplib_nope=`git rev-parse master`
cd ..
mkdir mainproj
cd mainproj
echo "(defproject mainproj \"0.1.0\" :dependencies [^{:voom {:repo \"../deplib\" :sha \"$deplib_yep\"}} [deplib \"0.1.0\"]])" > project.clj
lein voom freshen # Works as expected
echo "(defproject mainproj \"0.1.0\" :dependencies [^{:voom {:repo \"../deplib\" :sha \"$deplib_nope\"}} [deplib \"0.1.0\"]])" > project.clj
lein voom freshen # No matching version found for: deplib/deplib {:allow-snaps false, :freshen true, :repo "../deplib", :sha "thesha"}

I can "fix" the issue by tweaking the file-path-merges function (see pin-to-merge-commits). AFAICT the way it currently is written it causes merge commits to have no files in the r-commit-path relation and consequently merge commits don't show up in the candidates-a query (though there is probably a reason it is this way). I also need to adjust the sha verification function to not skip past the merge commit sha.

There could be a reason Voom doesn't work w/ merge commits; just thought I'd open an issue as I found it a bit unexpected.

[abrooks]

@takeoutweight Thanks for the detailed report. I'll take a look shortly but suspect you're correct.

[abrooks]

@takeoutweight You clearly put in a lot of effort in to both the reproducer and the patch — thank you!

The behavior of freshen in this case is correct but, perhaps not as helpful as it could be. The merge case you show introduces no new changes so the merge is, indeed, not a fresher commit than the feature-branch commit. You would see different behavior if you add the following actions before the merge:

...
git checkout master # same one, for context
touch filea && git add filea && git commit -m "add filea"
...

Voom does work fine with merge commits but will only identify them as freshen candidates if they either (A) introduce new changes (which isn't a good practice aside from merge collisions) or (B) introduce changes within/below the project directory that haven't seen each other before.

What we fail to do is be helpful. If a :sha is specified, I think we should take that as an override and generate an appropriate voom version for you. I have some changes in mind to newest-voom-ver-by-spec which will apply a specified sha as the result.

Does this make sense?

[abrooks]

I should also note that the proposed changes would actually make /any/ merge a candidate, even in cases where nothing changes on one of the merge parents. That's not the behavior we want.

Again, thank you for putting so much effort in to this! I'll try to provide the :sha override feature soon.

[takeoutweight]

Yes, that explanation makes sense, thanks! I had wondered if something like that might be the case. I definitely have a better mental model of freshen now.

[abrooks]

Having any mental model of freshen is quite an accomplishment. I think @Chouser and I agree that it's some of the most mind-warping code we've ever written. If you look through the lein-voom commit history you'll see the abandoned attempts at getting freshen both correct and performant.

I'm perpetually ashamed when someone takes the time to actually dig in to the voom code. What a pile of horror.