search

nvaccess / nvda

NVDA, the free and open source Screen Reader for Microsoft Windows
Other
2.01k stars 624 forks source link

Reconsider fast_diff_match_path license violation workaround #16633

Open LeonarddeR opened 3 weeks ago

LeonarddeR commented 3 weeks ago

Is your feature request related to a problem? Please describe.

I have found some oddities in UnicodeNormalizationOffsetConverter that are related to difflib doing very odd things on the character level. I haven't been able to reproduce this with non-private content, though. Ideally I'd move to fast_diff_match_patch, but I don't want to change nvda_DMP just for this. This made me question the decision to create NVDA_DMP in the first place. Has this been discussed extensively in the past and if so, where?

Also note that from a python console, it is still possible to import fast_diff_match_patch. I wonder if this is actually allowed within the licensing provisions. There is some inconsistency here.

Describe the solution you'd like

I know about license compatibilities, but I'm also reading from Apache:

Despite our best efforts, the FSF has never considered the Apache License to be compatible with GPL version 2, citing the patent termination and indemnification provisions as restrictions not present in the older GPL license. The Apache Software Foundation believes that you should always try to obey the constraints expressed by the copyright holder when redistributing their work.

Long story short:

  1. The incompatibility problem is not Apache's fault
  2. The exceptions stated by the NVDA license should allow us to use FDMP from NVDA core directly when added as a GlobalPlugin bundled with core, because:
    • It does not prevent such a plugin from being licensed under the terms of the GPL v2
    • The component (i.e. FDMP) doesn't use NVDA internals itself)
    • If FDMP is only used by the plugin, the plugin can operate as a bridge between FDMP and NVDA

By the way, the license actually allows us to stretch the term plugin in the developer guide to such an extent that we can also classify parts of the core that stand alone but are not necessarily globalPlugins as plugins, making them subject to this license provision.

Describe alternatives you've considered

Refactor nvda_dmp in such a way that it returns the full diff, rather than only insertions.

Any thoughts @codeofdusk, @Danstiv

Originally posted by @LeonarddeR in https://github.com/nvaccess/nvda/issues/11548#issuecomment-2139287528

Danstiv commented 3 weeks ago

I don't understand why dmp should return full diff? What problem will this solve?

LeonarddeR commented 3 weeks ago

For textUtils.UnicodeNormalizationOffsetConverter, I need a full diff because I need to know what exactly was replaced by normalization.

Danstiv commented 3 weeks ago

This is definitely possible, but I don't know how much it will degrade performance, it might be worth considering using a fast diff match patch in the core directly.

LeonarddeR commented 3 weeks ago

I've been able to work around the issues in UnicodeNormalizationOffsetConverter, so it turns out i can rely on difflib and difflib is even offering some extra information fatch_diff_match_patch doesn't. So that takes the pressure off as far as I'm concerned. That said, it is still possible to do import fast_diff_match_patch from an installed NVDA. That makes fast_diff_match_patch directly available from GPL code. Cc @seanbudd , @gerald-hartig for triage. I think it should be investigated whether the GPL is violated here.

seanbudd commented 3 weeks ago

We don't have plans/capacity to investigate this in the near future. Considering this is not blocking any technical/product changes a legal investigation is not a priority