nvaccess / nvda

NVDA, the free and open source Screen Reader for Microsoft Windows
https://www.nvaccess.org/
Other
2.11k stars 637 forks source link

scons: Fix changing order of dependencies when building an interface with multiple IDL files #7036

Open jcsteh opened 7 years ago

jcsteh commented 7 years ago

We've noticed for some time that certain IDL files tend to get rebuilt even when they haven't changed. While very minor in the grand scheme of things before now, it is a real problem with unit testing (#7026) because these files can be rebuilt when you just want to run tests.

The two IDLs in question are ISimpleDOMNode and MathPlayerDLL, both of which import/include additional IDL files. This does not happen for AcrobatAccess or IA2, both of which only have a single IDL file.

Here are my findings after several hours of investigation.

This unnecessary rebuild only happens once; i.e. the second run of scons. While this doesn't seem so serious, it is still a problem for unit testing because we run scons source and scons tests separately in AppVeyor and we don't want the unnecessary rebuild for scons tests.

The easiest way to trigger this without rebuilding the entire tree is to remove build\x86\ISimpleDOM* and then run scons source build\x86\ISimpleDOMNode.h.

The reason it happens is due to the order of dependencies changing:

scons build\x86\ISimpleDOMNode.h --debug=explain
...
scons: rebuilding `build\x86\ISimpleDOMNode.tlb' because the dependency order changed:
              old: ['build\\x86\\ISimpleDOMNode.idl', 'C:\\Program Files (x86)\\Microsoft SDKs\\Windows\\v7.1A\\bin\\MIDL.EXE', 'build\\x86\\ISimpleDOMDocument.idl', 'build\\x86\\ISimpleDOMText.idl']
               new: ['build\\x86\\ISimpleDOMNode.idl', 'build\\x86\\ISimpleDOMDocument.idl', 'build\\x86\\ISimpleDOMText.idl', 'C:\\Program Files (x86)\\Microsoft SDKs\\Windows\\v7.1A\\bin\\MIDL.EXE']

So, scons realises that building the .idl requires midl.exe. However, in the first run, midl comes before the IDL dependencies, but in the second run, it comes after.

The following attempts to fix this did not work; the order of dependencies was still different:

  1. Explicitly declaring ISimpleDOMDocument.idl and ISimpleDOMText.idl as dependencies of ISimpleDOMNode.idl.
  2. Declaring ISimpleDOMDocument.idl and ISimpleDOMText.idl as additional sources to the midl tool.
  3. Declaring ISimpleDOMDocument.idl and ISimpleDOMText.idl as dependencies of ISimpleDOMNode.tlb, ISimpleDOMNode.h, etc.
  4. Using env.Install instead of env.Command with Copy to copy the files from miscDeps into the build dir. (We've had problems with Copy misbehaving in strange ways previously.)

Here's some conjecture as to why the order of dependencies changes:

I'm pretty sure this is a bug/quirk in scons; I don't see how we can do anything differently here.

For now, I've hacked around this (266c7513) by instructing scons to ignore midl as a dependency when determining whether a rebuild is necessary. Ideally, we'd get this fixed in scons, but the necessary legwork is unfortunately not something we have time for at this stage. I'm opening this issue to document my findings in case we/someone else wants to pursue this in future.

Adriani90 commented 5 years ago

cc: @feerrenrut, @francipvb

Adriani90 commented 1 year ago

cc: @seanbudd

Adriani90 commented 1 month ago

cc: @gerald-hartig, @SaschaCowley for triaging.

SaschaCowley commented 6 hours ago

@jcsteh are you still able to reproduce this? I've attempted to repro this morning and don't think I'm experiencing the issue.

First, I commented out your workaround in ISimpleDOM_sconscript. https://github.com/nvaccess/nvda/blob/511e7221899ed9cf7ce2a6c4fd20c1a9101ad867/nvdaHelper/ISimpleDOM_sconscript#L49-L51

Then, from a clean clone:

PS D:\projects\nvda> ./scons source
Ensuring NVDA Python virtual environment
Virtual environment does not exist.
Creating virtual environment...
# ...
call py -m SCons source
scons: Reading SConscript files ...
Building with 16 concurrent jobs
scons: done reading SConscript files.
scons: Building targets ...
# ...
scons: done building targets.
Deactivating NVDA Python virtual environment
PS D:\projects\nvda> del build\x86\ISimpleDOM*.idl
PS D:\projects\nvda> ./scons source -j1
Ensuring NVDA Python virtual environment
call py -m SCons source -j1
scons: Reading SConscript files ...
Warning: Building with 1 concurrent job while 16 CPU threads are available. Running SCONS with the parameter '-j16' may lead to a faster build. Running SCONS with parameter '--all-cores' will automatically pick all available cores.
scons: done reading SConscript files.
scons: Building targets ...
Copy("build\x86\ISimpleDOMText.idl", "miscdeps\include\ISimpleDOM\ISimpleDOMText.idl")
Copy("build\x86\ISimpleDOMDocument.idl", "miscdeps\include\ISimpleDOM\ISimpleDOMDocument.idl")
Creating 'build\x86\iSimpleDOMNode.idl'
Install file: "include\espeak\dictsource\extra\cmn_listx" as "include\espeak\dictsource\cmn_listx"
scons: `source' is up to date.
scons: done building targets.
Deactivating NVDA Python virtual environment

If I delete all ISimpleDOM* files from build\x86, this causes a number of files to be rebuilt, but this makes sense as the DLLs are missing, and then are identified as different because the timestamps don't match.

I'm not sure if I'm missing something, or if an update to SCons has fixed this.

jcsteh commented 4 hours ago

I'm not sure. If you can't reproduce it and it's not causing problems for other users or CI, I'm happy for this to just be closed. I can always reopen if I encounter this again.