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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build NVDA with Visual Studio 2019 #10169

Merged
merged 10 commits into from Feb 13, 2020
Merged

Conversation

leonardder
Copy link
Collaborator

@leonardder leonardder commented Sep 6, 2019

Note, this is a draft pr. It is currently not planned to receive any attention before NVDA 2019.3 is released.

Link to issue number:

Fixes #9446

Summary of the issue:

There are several reasons to update to Visual Studio 2019:

  • Liblouis now requires a non MSVC compiler to build. Visual Studio 2019 bundles Clang and therefore, updating to liblouis 3.11 after that will be relatively easy
  • It offers a more accessible IDE. Therefore, many people might have the desire to update and are reluctant to do so.
  • It is newer, and newer usually means better 馃槀

Description of how this pull request fixes the issue:

  • Updates scons to version 3.1.1 which supports vs2019
  • Changed appveyor.iml to build with the Visual Studio 2019 image
  • Update the scons build environment to ensure vs2019 is used

Testing performed:

Tested locally that scons source runs correctly.

Known issues with pull request:

  • There is one appveyor related issue. It is not able to sign builds using the authenticode certificate.
  • Linting fails. I'm afraid that the scons submodule update makes it fails somehow. I tried running flake8 with more verbosity, but apart from creating a very huge error log with nothing special in it, it doesn't say anything about the cause of the failure.

Change log entry:

  • Changes for developers
    • Compiling NVDA dependencies now requires Visual Studio 2019 (16.2 or newer).
    • Updated SCons to version 3.1.1

@AppVeyorBot
Copy link

@AppVeyorBot AppVeyorBot commented Sep 17, 2019

PR introduces Flake8 errors 馃槻

See test results for Failed build of commit 19010f29db

Leonard de Ruijter added 2 commits Sep 18, 2019
@AppVeyorBot
Copy link

@AppVeyorBot AppVeyorBot commented Sep 18, 2019

PR introduces Flake8 errors 馃槻

See test results for Failed build of commit 915c5f8752

@michaelDCurran
Copy link
Member

@michaelDCurran michaelDCurran commented Sep 19, 2019

Was the only issue with this pr the comInterface_sonscript stuff, which has already been merged to master, or was there another issue... something about certificate signing?
My personal view is that we should research switching to vs 2019 for 2019.3 as much as possible, with the aim of then upgrading Liblouis to an official release again. However I do know @feerrenrut has some concirns around making too many changes in this release. At very least I think we need to see it working in action before we can say how bigger change it really is, and then debate if we should merge it. I think the argument that vs 2017 is now no longer the current version and is harder to get is a valid one.

@leonardder
Copy link
Collaborator Author

@leonardder leonardder commented Sep 19, 2019

@leonardder
Copy link
Collaborator Author

@leonardder leonardder commented Sep 19, 2019

The following entries from the appveyor build log of a try snapshot are relevant:

[00:00:14] if(!$env:APPVEYOR_PULL_REQUEST_NUMBER) {
[00:00:14]  openssl enc -aes-256-cbc -d -pass pass:$env:encFileKey -in authenticode.pfx.enc -out authenticode.pfx
[00:00:14]  openssl enc -aes-256-cbc -d -pass pass:$env:encFileKey -in ssh_id_rsa.enc -out ssh_id_rsa
[00:00:14]  # Install ssh stuff.
[00:00:14]  copy ssh_id_rsa $env:userprofile\.ssh\id_rsa
[00:00:14] }
[00:00:14] 
[00:00:15] openssl : *** WARNING : deprecated key derivation used.
[00:00:15] At line:2 char:2
[00:00:15] +  openssl enc -aes-256-cbc -d -pass pass:$env:encFileKey -in authentic ...
[00:00:15] +  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[00:00:15]     + CategoryInfo          : NotSpecified: (*** WARNING : d...erivation used.:String) [], RemoteException
[00:00:15]     + FullyQualifiedErrorId : NativeCommandError
[00:00:15]  
[00:00:15] Usi
[00:00:15] ng -iter or -pbkdf2 would be better.
[00:00:15] 
[00:00:15] b
[00:00:15] ad decrypt
[00:00:15] 
[00:00:15] 2576
[00:00:15] 99
[00:00:15] 02
[00:00:15] 14
[00:00:15] 4:
[00:00:15] erro
[00:00:15] r:
[00:00:15] 06065
[00:00:15] 06
[00:00:15] 4:dig
[00:00:15] it
[00:00:15] al envel
[00:00:15] op
[00:00:15] e rou
[00:00:15] tin
[00:00:15] es
[00:00:15] :E
[00:00:15] VP
[00:00:15] _Decr
[00:00:15] yp
[00:00:15] tFi
[00:00:15] n
[00:00:15] al
[00:00:15] _e
[00:00:15] x:
[00:00:15] bad d
[00:00:15] ecrypt:crypto/evp/evp_
[00:00:15] enc.c:537:
[00:00:15] 
[00:00:15] 
[00:00:15] openssl : *** WARNING : deprecated key derivation used.
[00:00:15] At line:3 char:2
[00:00:15] +  openssl enc -aes-256-cbc -d -pass pass:$env:encFileKey -in ssh_id_rs ...
[00:00:15] +  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[00:00:15]     + CategoryInfo          : NotSpecified: (*** WARNING : d...erivation used.:String) [], RemoteException
[00:00:15]     + FullyQualifiedErrorId : NativeCommandError
[00:00:15]  
[00:00:15] Using -iter or -pbkdf2 would be better.
[00:00:15] bad decrypt
[00:00:15] 25769902144:error:06065064:digital envelope routines:EVP_DecryptFinal_ex:bad decrypt:crypto/evp/evp_enc.c:537:
[00:02:19] signtool sign /fd SHA256 /f appveyor\authenticode.pfx /t http://timestamp.digicert.com C:\projects\nvda\build\x86\IAccessible2proxy.dll
[00:02:20] SignTool Error: An error occurred while attempting to load the signing

[00:02:20] 	certificate from: appveyor\authenticode.pfx

[00:02:20] scons: *** Error 1

@michaelDCurran
Copy link
Member

@michaelDCurran michaelDCurran commented Sep 20, 2019

The flake8 issue can be fixed by changing a line in tests/lint/flake8.ini.
On line 43, change:

include,

to

include/*,

In this pr, the scons submodule has changed, and therefore git diff lists include/scons as being changed. However, it seems that flake8 was not correctly matching include to include/scons. Looks like they need to match fully. thus include/* will cover any changes to submodules in include now.
As this is a draft pr, I was unable to push a commit to address this.

@michaelDCurran
Copy link
Member

@michaelDCurran michaelDCurran commented Sep 20, 2019

The vs2017 appveyor image includes openssl 1.0.2J where as vs2019 appveyor image contains openssl 1.1.1a.
It seems there is a change in openssl that no longer allows decrypting the way we used to.
Am doing further research.

@michaelDCurran
Copy link
Member

@michaelDCurran michaelDCurran commented Sep 20, 2019

openssl 1.1.0 changed its digest algorithm for deriving the key from md5 to sha256. For now, we can supply the -md md5 argument in our openssl commands in appveyor.yaml.

openssl enc -md md5 -aes-256-cbc -d -pass pass:$env:encFileKey -in authenticode.pfx.enc -out authenticode.pfx
    openssl enc -md md5 -aes-256-cbc -d -pass pass:$env:encFileKey -in ssh_id_rsa.enc -out ssh_id_rsa

Reference: https://stackoverflow.com/questions/39637388/encryption-decryption-doesnt-work-well-between-two-different-openssl-versions

@leonardder
Copy link
Collaborator Author

@leonardder leonardder commented Sep 20, 2019

Thanks for finding all this out.

  • Regarding flake8, it turns out that excludes tells flake8 not to traverse into a directory, but if a file is provided in a diff, it will still check that file unless it is explicitly excluded, either with the file name or with a wild card. For now, I only changed include into include/*. If we ever change something in miscDeps, I don't think that will cause a problem, as only miscDeps will be in the diff and that's already excluded. Other excludes won't be in the diff as they are already excluded at the git level.

  • Openssl: I changed this and pushes another try build

@leonardder
Copy link
Collaborator Author

@leonardder leonardder commented Sep 20, 2019

Actually, before getting in a rush, it might be good to point out that @egli filed an issue with the idea to port liblouis to rust. In that case, we will have to use binary builds anyway. In liblouis/liblouis#704 (comment), I asked whether there is a time frame for this.

having said that, I don't think it will happen for the next few releases.

@leonardder leonardder marked this pull request as ready for review Sep 20, 2019
@XLTechie
Copy link
Contributor

@XLTechie XLTechie commented Sep 20, 2019

@michaelDCurran
Copy link
Member

@michaelDCurran michaelDCurran commented Sep 20, 2019

@leonardder
Copy link
Collaborator Author

@leonardder leonardder commented Sep 23, 2019

@michaelDCurran: re liblouis, @egli pointed out that liblouis in rust won't be there any time soon. It is more of a wish, currently.

@dpy013
Copy link
Contributor

@dpy013 dpy013 commented Nov 10, 2019

hi
Appveyor updated Windows images update on November 8, 2019
The visual studio2019 version has been updated to 16.3.8

@leonardder leonardder added this to the 2020.1 milestone Jan 14, 2020
@leonardder
Copy link
Collaborator Author

@leonardder leonardder commented Jan 21, 2020

@feerrenrut and @michaelDCurran, as you started merging in pull requests for 2020.1, I think this is a candidate to deserve priority, as #10275 relies on it which would benefit from an extensive testing period.

@feerrenrut
Copy link
Member

@feerrenrut feerrenrut commented Jan 23, 2020

@leonardder Yes I agree, I'll take a look at this soon.

@michaelDCurran
Copy link
Member

@michaelDCurran michaelDCurran commented Feb 13, 2020

As there are no extraordinary problems resulting from NVDA 2019.3, I think it is now safe to merge this to master, in order to give it a good run before a beta.

@michaelDCurran michaelDCurran merged commit 67ca5c4 into nvaccess:master Feb 13, 2020
1 check passed
michaelDCurran added a commit that referenced this issue Feb 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants