Replace Platform.IsMono with actual OS checks

sillsdev / chorus

End-user collaboration library via 3-way xml merging and hg dvcs under the hood

6 stars 26 forks source link

Replace Platform.IsMono with actual OS checks #356

Closed rmunn closed 2 weeks ago

rmunn commented 4 weeks ago

Fix #355.

Some cases of Platform.IsMono are workarounds for bugs in older versions of mono. Those can probably be removed, but for safety's sake, we'll leave them alone for now.

Other cases are clearly trying to check the platform, to run Linux-only or Windows-only code. Those cases should be replaced by Platform.IsUnix or Platform.IsWindows as appropriate, because nearly everywhere that Chorus is being run on Linux these days, it's running under dotnet rather than mono, and the Platform.IsMono check returns false.

This change is

rmunn commented 4 weeks ago

@hahn-kev - This bug is "in the wild" in LfMerge. If LfMerge ever encounters a merge conflict between two .txt files, it will try to run Windows-only code and promptly error out, resulting in a Mercurial rollback because the merge conflict wasn't handled. Thankfully it's extremely unlikely that anyone will check .txt files into their FieldWorks project, let alone end up with a merge conflict in those files. However, I want to prioritize reviewing and merging this bugfix so that I can push a new LfMerge release that won't try to run Windows-only code.

github-actions[bot] commented 4 weeks ago

Test Results

      4 files ±0   412 suites ±0 2h 45m 51s :stopwatch: +5s   883 tests ±0   860 :heavy_check_mark: ±0   23 :zzz: ±0 0 :x: ±0 4 040 runs ±0 3 906 :heavy_check_mark: ±0 134 :zzz: ±0 0 :x: ±0

Results for commit 4932740a. ± Comparison against base commit a6a70411.

:recycle: This comment has been updated with latest results.