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

FixFwData now does real work #1159

Merged
merged 3 commits into from
Oct 25, 2024
Merged

FixFwData now does real work #1159

merged 3 commits into from
Oct 25, 2024

Conversation

rmunn
Copy link
Contributor

@rmunn rmunn commented Oct 25, 2024

Fix #1155.

Our FixFwData program was a do-nothing implementation, there only because FlexBridge requires a program called FixFwData to exist and be runnable. This was fine as long as the only Send/Receives we were triggering in Lexbox code were in integration tests, where no merge conflicts could happen. However, once we start allowing CrdtMerge to do real Send/Receives of user data, merge conflicts can happen, and FixFwData is there to take the merge decisions made by ChorusMerge and fix them when they would produce invalid .fwdata files. (For example, two people might add the same part of speech to a project, and the merge resolution would keep both. That would be valid XML but invalid .fwdata because they would both have the same GUID, so FixFwData would keep just one of the two parts of speech with that GUID).

The guts of FixFwData are provided by liblcm, so all we have to do here is write a wrapper program. I've chosen to copy the LfMerge implementation, and change its LoggingProgress implementation to use Microsoft's ILogger interface instead of Console.WriteLine. Which means, since I copied the implementation from LfMerge's LGPL'ed implementation, that this FixFwData implementation is also LGPL'ed. Since it's a separate .exe from the rest of our code, this has no implications for the license of the rest of the Lexbox code, which is MIT licensed.

@rmunn rmunn self-assigned this Oct 25, 2024
@rmunn rmunn requested a review from hahn-kev October 25, 2024 02:39
Copy link

github-actions bot commented Oct 25, 2024

C# Unit Tests

75 tests  ±0   75 ✅ ±0   5s ⏱️ ±0s
13 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit 02b7bfc. ± Comparison against base commit 4b27266.

♻️ This comment has been updated with latest results.

@rmunn
Copy link
Contributor Author

rmunn commented Oct 25, 2024

Looks like #1161 might need to be merged first in order to resolve the NuGet package downgrade warnings.

@rmunn
Copy link
Contributor Author

rmunn commented Oct 25, 2024

Going to change the PR target to the #1161 branch so that I can get this to build. GitHub will auto-switch the target back to develop once #1161 is merged, at which point I'll just need to rebase.

@rmunn rmunn changed the base branch from develop to chore/bump-dependencies-for-system-text-json October 25, 2024 03:51
@rmunn rmunn force-pushed the feat/real-fixfwdata branch from c1b2843 to f933b2b Compare October 25, 2024 03:54
@rmunn
Copy link
Contributor Author

rmunn commented Oct 25, 2024

Wow, I have managed to completely confuse GitHub's PR checks. It is giving me green checkmarks even though no checks have run, and even says that no review is required for this PR!

what-do-you-mean-no-review-required

I'm going to close and reopen the PR, since I know that tends to reset its status.

@rmunn rmunn closed this Oct 25, 2024
@rmunn rmunn reopened this Oct 25, 2024
@rmunn rmunn force-pushed the feat/real-fixfwdata branch from ffb59e5 to f933b2b Compare October 25, 2024 03:59
@rmunn
Copy link
Contributor Author

rmunn commented Oct 25, 2024

No, I see. It doesn't run checks on PRs that aren't against develop. I'm going to have to change the PR's target back to develop.

@rmunn rmunn changed the base branch from chore/bump-dependencies-for-system-text-json to develop October 25, 2024 04:00
@rmunn rmunn force-pushed the feat/real-fixfwdata branch from ffb59e5 to f933b2b Compare October 25, 2024 04:01
rmunn added 3 commits October 25, 2024 16:17
When running `dotnet restore` on Linux against the whole solution, it
fails with the following error message:

error NETSDK1100: To build a project targeting Windows on this operating system, set the EnableWindowsTargeting property to true.

We don't want to set EnableWindowsTargeting on Mac, so we'll use a
condition to only set it when building on Linux.
FixFwData uses the latest version of liblcm, which needs a later
SIL.Core release than the one Testing.csproj is currently targeting.
@rmunn rmunn force-pushed the feat/real-fixfwdata branch from ffac124 to 02b7bfc Compare October 25, 2024 09:17
@rmunn rmunn enabled auto-merge (squash) October 25, 2024 09:20
@rmunn rmunn merged commit 6a0d473 into develop Oct 25, 2024
9 checks passed
@rmunn rmunn deleted the feat/real-fixfwdata branch October 25, 2024 09:22
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.

Make FixFwData do real work
2 participants