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

PropertyRule always overwrites with THEIR or OUR even if there is no conflict on the given property #21

Open
harrisric opened this issue Oct 6, 2021 · 4 comments

Comments

@harrisric
Copy link

If we have the scenario as follows:

  • Base {A = 1, B = 1}
  • BranchX {A = 2, B = 1}
  • BranchY {A = 1, B = 2}

Then we would expect;

  • merge of BranchX (source) into BranchY (target) with strategy THEIRS to result in {A=2, B=2}, but actually results in {A=2, B=1}
  • merge of BranchX (source) into BranchY (target) with strategy OURS to result in {A=1, B=2} (which it currently does).

Given that the basePom is available to PropertyRule it seems like this is a straightforward condition check. At the moment it looks like it simply overwrites the property regardless of whether it has changed.

@cecom
Copy link
Owner

cecom commented Oct 6, 2021

Both of your scenarios are working as intended. With the PropertyRule you define properties which should not be merged in the "usual" git way. You define it as that "our"/"their" ALWAYS wins, regardless of the base pom/source/target values.

What you want here, is the normal git merge algorithm. If you want that, you should not include Property A and B in the PropertyRule yaml definition.

@harrisric
Copy link
Author

Thanks for the quick response!
In this simple example we do want the basic git merge behaviour, however where there is a genuine conflict between the properties then we want the incoming to win. If we rely on the standard git merge then this will be flagged as a conflict and need manual resolution.
i.e.
Base {A = 1, B = 1}
BranchX {A = 3, B = 1}
BranchY {A = 2, B = 2}
merge of BranchX (source) into BranchY (target) with strategy THEIRS to result in {A=3, B=1} with the rule (which is still not what we want - we would like A = 3, B = 2), but without the rule would leave us with a conflict to manually resolve.

@cecom
Copy link
Owner

cecom commented Oct 6, 2021

This would be a new feature. Want you want is:

if there is no conflict do a normal merge, if there is a conflict use the OURS/THEIRS value.

Due to the fact, that git only calls the merge driver if there IS a conflict your scenario only triggers if there is a minimum of 2 property changes.

A solution could be a flag aka "conflictMode" e.G.:

--- !!de.oppermann.pomutils.rules.PropertyRule
    strategy: OUR                 # possible values: OUR|THEIR
    onlyWhenConflicting:  TRUE    # defaults to FALSE
    properties:                   # a list of property names
       - foobar

If the "onlyWhenConflicting" flag is set, than the value of "strategy" is considered when there is a conflict. Otherwise we look at the Base/Their/Our pom value and take the value which would be taken if it would be a normal merge.

@harrisric
Copy link
Author

harrisric commented Oct 6, 2021

Sounds like a good plan.
I've managed to change a local version to do this without the flag. No existing tests fail, so it wasn't explicitly looked at, but I have added new assertions to the existing tests and amended the resources in such a way that I can check the new behaviour.

I'll try and work out where the flag might fit in.

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

No branches or pull requests

2 participants