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

Fixes up a commit adjacent to the line being changed #15

Open
nickolay opened this issue Jul 14, 2019 · 1 comment
Open

Fixes up a commit adjacent to the line being changed #15

nickolay opened this issue Jul 14, 2019 · 1 comment
Labels
help wanted Extra attention is needed

Comments

@nickolay
Copy link
Contributor

After fixing #6, I noticed another issue with the testcase from that issue.

In the testcase the commit to absorb is removing all instances of #[cfg(feature = "rsa_keygen")] and there is an intermediate commit adding a function with that attribute:

83      }
84   
85      #[cfg(feature = "rsa_keygen")]
86   +  pub fn limbs_even_add_one(r: &mut [Limb]) {
     +      // ...
91   +  }
92   +  
93   +  #[cfg(feature = "rsa_keygen")]
94      pub fn limbs_count_low_zero_bits(v: &[Limb]) -> u16 {

git-absorb decides to fix up this intermediate commit with both the hunk removing the cfg attribute at line 85 and at line 93, while arguably line 85 removal doesn't belong to the intermediate commit, as line 85 wasn't added by the commit.

It's clear why git-absorb does it, and if it produced a fixup for an older commit introducing line 85, that fixup would conflict with the intermediate commit. Still, git-absorb could:

  • Keep the removal of line 85 in the index
  • Warn about this
  • It's also possible rewrite the history to remove line 85 from an older commit, if we don't limit ourselves to producing fixup commits on top of existing history, but that's probably out of scope for git-absorb?
@tummychow
Copy link
Owner

hmm... a very interesting problem. sounds like the desired behavior here would "don't absorb this hunk into this commit, if doing so would widen the range of lines affected by the commit".

i'd probably put this behavior and the current behavior behind flags (strawperson names: --conservative for this, --aggressive for the existing behavior). the default behavior would be the more conservative option, since you can always rerun with more aggressive behavior afterwards. it would leave the hunk in the index while printing an appropriate log line.

i also wonder if this case can be handled better by hg-absorb's linelog-based strategy, but that's a much larger and more complicated consideration

@tummychow tummychow added the help wanted Extra attention is needed label Jul 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants