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

Update to recent version of liblouis #10161

Closed
leonardder opened this issue Sep 2, 2019 · 8 comments · Fixed by #10275
Closed

Update to recent version of liblouis #10161

leonardder opened this issue Sep 2, 2019 · 8 comments · Fixed by #10275
Assignees
Labels
blocked component/liblouis
Milestone

Comments

@leonardder
Copy link
Collaborator

@leonardder leonardder commented Sep 2, 2019

We will have to update to liblouis 3.11 to properly support output of unicode surrogate characters. Unfortunately, liblouis 3.11 which just came out has build errors, see liblouis/liblouis#838

This issue can be tracked for the liblouis 3.11 update. I will file a pr as soon as there is a fix upstream.

@leonardder leonardder added the component/liblouis label Sep 2, 2019
@leonardder leonardder self-assigned this Sep 2, 2019
@leonardder leonardder added this to To do in Update NVDA to Python 3 via automation Sep 2, 2019
@leonardder
Copy link
Collaborator Author

@leonardder leonardder commented Sep 5, 2019

I've discussed this matter with @DaveMielke in this thread.

In short, Liblouis introduced variable length array in its code, which is a construct in C99, and updated specification of the C language. These things are not supported in the C compiler delivered with Visual Studio, and therefore we won't be able to build liblouis as we do now.

Research and discussion with @feerrenrut revealed the following two solutions:

  1. Update NVDA build environment to use Visual Studio 2019, which has a component for the clang compiler. Compile liblouis with the Clang compiler, which is compatible with MSVC and supports C99's variable length arrays. I prefer this approach, as it is probably the easiest to implement (I basically have a prototype for this). A downside is that people who compile NVDA locally have to update their Visual Studio. However, as Visual Studio Community is a free product and it has accessibility improvements in almost every release, I think there aren't that many people having difficulties with that.
  2. Provide a binary build in a binary repository, as we do with comtypes, py2exe and WXPython. A major downside is that this requires more work for every update of liblouis, it is harder to provide test builds.

As an in-between solution, we could stick to liblouis 3.11 for NVDA 2019.3 and than switch to 3.12 + VS2019 in 2020.

@dkager: also curious about your opinion.

@lukaszgo1
Copy link
Contributor

@lukaszgo1 lukaszgo1 commented Sep 5, 2019

@leonardder Wrote in part:

As an in-between solution, we could stick to liblouis 3.11 for NVDA 2019.3 and than switch to 3.12 + VS2019 in 2020.

I am personally against this. There is no reason not to upgrade to VS 2019 - it even still support Windows 7, so no one is disadvantaged, and delaying it might cause users not to experience improvements in the braille tables in never Liblouis releases.

@leonardder
Copy link
Collaborator Author

@leonardder leonardder commented Sep 6, 2019

I assume this will at some point be discussed by @michaelDCurran and @feerrenrut

@leonardder
Copy link
Collaborator Author

@leonardder leonardder commented Sep 6, 2019

I am personally against this. There is no reason not to upgrade to VS 2019 - it even still support Windows 7, so no one is disadvantaged, and delaying it might cause users not to experience improvements in the braille tables in never Liblouis releases.

Liblouis will always improve, and there will always be a reason to update. However, updating to Visual Studio 2019 causes a major dependency change, and we already had plenty of them during the Python 3 migration.

Unless you are suffering from a critical liblouis issue that has high priority, I think it is the most safe to stick to 3.10 for a while.

@lukaszgo1
Copy link
Contributor

@lukaszgo1 lukaszgo1 commented Sep 6, 2019

@leonardder No, I don't. Having said that there are few reasons to update to VS 2019 sooner than later.

  1. Using a particular commit of Liblouis rather than full release mean that any potential issue fixed after the commit we are using, and before the release would be affecting our users on 2019.3. For many Python 3 migration means loss of favorite add-ons and we should make upcoming version as stable as possible not to discourage users.
  2. As VS 2017 isn't the latest it is currently not as easy as it should be to download it for new developers. You have to create VS dev essential account and look for the older version on a separate web page.
  3. If someone has VS 2019 already installed and he wants to contribute to NVDA he has to remove VS 2019, install 2017 and reinstall 2019 as it is required to install these versions in order of release.
  4. if we update we also has to upgrade SCons which would fix #9990. I honestly do not understand why we cannot do this before VS update if it allows someone to build, but given the fact that you've closed it as a duplicate of the issue talking about update to 2019 apparently we cannot.

I don't see what waiting would bring here. The only group for whom it would be a small annoyance are existing developers.

@leonardder
Copy link
Collaborator Author

@leonardder leonardder commented Sep 7, 2019

1. Using a particular commit of Liblouis rather than full release mean that any potential issue fixed  after the commit we are using, and before the release would be affecting our users on 2019.3. For many Python 3 migration means loss of favorite add-ons and we should make upcoming version as stable as possible not to discourage users.

One of the things about liblouis that concern me a bit, is that many features in liblouis are merged only just hours or days before the release. So I don't think that you can strictly say that a release version is definitely more stable than a random commit before the release of that particular version. In fact, this commit only contains bug fixes since 3.10, no new features.

2. As VS 2017 isn't the latest it is currently not as easy as it should be to download it for new developers. You have to create VS dev essential account and look for the older version on a separate web page.

This makes a lot of sense. I'd like to have @feerrenrut's opinion about this. We should make it as smooth as possible for a new developer to step into the NVDA development process.
In fact, it is perfectly possible to use visual studio 2019 to compile NVDA already, I do it myself. You can just install VS 2019 and enable the installation of the 2017 build tools in the optional section of the c++ workload.

3. If someone has VS 2019 already installed and he wants to contribute to NVDA he has to remove VS 2019, install 2017 and reinstall 2019 as it is required to install these versions in order of release.

As per above comment, this is not necessary, but may be we should explain that somewhere.

4. if we update we also has to upgrade SCons which would fix #9990. I honestly do not understand why we cannot do this before VS update if it allows someone to build, but given the fact that you've closed it as a duplicate of the issue talking about update to 2019 apparently we cannot.

If we update to SCons 3.1.1 without updating visual studio, we actually make it impossible for 2019 users to build NVDA with it, so we would make the current situation worse and stop users (like me) to use the buildtools workaround.

I don't see what waiting would bring here. The only group for whom it would be a small annoyance are existing developers.

It ensures stability. Every change/update in build tools brings us a possible risk of instability. Having said that, appveyor updates visual studio once in a while, and therefore the build tools are also updated from time to time, so I'm aware that this point is not very strong.

@zstanecic
Copy link
Contributor

@zstanecic zstanecic commented Sep 7, 2019

@lukaszgo1
Copy link
Contributor

@lukaszgo1 lukaszgo1 commented Sep 7, 2019

@leonardder wrote:

In fact, it is perfectly possible to use visual studio 2019 to compile NVDA already, I do it myself. You can just install VS 2019 and enable the installation of the 2017 build tools in the optional section of the c++ workload.

And the chance that someone not very familiar with Visual Studio would know that are almost zero, so this should be documented.
Documenting this however creates two possible environments in which developers are able to build, so more chance for non standard behavior. I would prefer only one officially approved build path.

If we update to SCons 3.1.1 without updating visual studio, we actually make it impossible for 2019 users to build NVDA with it, so we would make the current situation worse and stop users (like me) to use the buildtools workaround.

At the moment the supported configuration is VS 2017. If someone like you wants to use never VS and is able to build successfully with it then why not. However by not following official build path I would expect that in case of any issues I am on my own. We are clearly stating that we support building with VS 2017, and we do not state that we aren't supporting users on 32-vit Windows versions. So we aren't upgrading SCons even we know that it would enable someone to build well because some developers decided to use never not officially approved build path and we would break building for them. I believe that you and almost everyone else should be able to install VS 2017, whereas reinstalling Windows to switch to 64-bit just to build NVDA isn't that easy, and in some cases simply not possible.

@feerrenrut feerrenrut added this to the 2019.4 milestone Oct 29, 2019
@feerrenrut feerrenrut removed this from To do in Update NVDA to Python 3 Oct 29, 2019
@leonardder leonardder changed the title Update to liblouis 3.11 Update to recent version of liblouis Dec 3, 2019
@michaelDCurran michaelDCurran removed this from the 2019.4 milestone Jan 13, 2020
@michaelDCurran michaelDCurran added this to the 2020.1 milestone Jan 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked component/liblouis
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants