If "No" is chosen, the requirements file is installed without recreating.

Can you explain a bit more for people like I who do not know the details of all this process? More specifically, does it mean that:

• if yes is chosen, the current virtual environment is deleted and a new one is completely recreated from scratch (today's process)
• if no is chosen, the existing environment is used and the dependencies are updated only if needed according to what has changed in the requirement file
  If this description is not correct, please update it.

This is correct.

@LeonarddeR I think we may be able to safely avoid recreating the venv at all if only requirements have changed, as long as we remove any old requirements no longer listed.
Take a look at the removeOldRequirements branch I just pushed to nvaccess/nvda.

I think this makes sense, however this does not remove dependencies of no longer needed packages. Could this be a problem?

I really don't know why pip has never supported better installed package dependency tracking.

That's a true thing. Pip is lacking many of these features that are pretty important to have. I think PipEnv would fill that gap, but I recall that has other issues, mainly related to performance.

If there are no other ideas then what is in this pr is suitable. I just want to double check the wording one more time.

I think this pr offers the best of both worlds by giving the user choice about what to do. Usually they can safely choose to keep the venv, we can always urge to a venv removal on NVDA Devel if it is strictly necessary.

But we may run into trouble when filtering nonDeps.txt as the packages listed in our requirements.txt may not be the exact most specific version possible.

An alternative approach would be not to use our requirements.txt as is but:

1. Create list of packages which are in our requirements file splitting them on the version specifiers.
2. List all packages installed in our virtual environment
3. and create a list of what we do not require based on the list created in point 1 but with version specifiers gathered in point 2.

So, I'm not sure if this is all worth it.

Agreed that this is a lot of work for a very questionable benefit.

I think this pr offers the best of both worlds by giving the user choice about what to do. Usually they can safely choose to keep the venv...

In what cases venv should be removed even though only packages are changed?

In what cases venv should be removed even though only packages are changed?

I must admit is this is a pretty hypothetical case. I'm not sure whether pip will handle cases properly where we downgrade a package in our requirements file that itself also specifies a downgrade of a dependant package.

In what cases venv should be removed even though only packages are changed?

I must admit is this is a pretty hypothetical case. I'm not sure whether pip will handle cases properly where we downgrade a package in our requirements file that itself also specifies a downgrade of a dependant package.

Also, there is of course the case where we specify a package upgrade that updates the package itself, but where the package doesn't specify strict version constraints for its dependencies. IN that case, at some point dependencies of dependencies won't be up to date and no longer equal to the versions used on appveyor.

Thanks for your explanation @LeonarddeR. In that case I would personally prefer to":

1. Uninstall all packages which were present in the old version of requirements which are no longer listed in the current requirements file
2. Uninstall unneeded dependencies using strategy outlined in the comments by @michaelDCurran and me above
3. Install only newly added dependencies from the requirements file.

Obviously it is possible that even with these improvements in place the version of requirements or some packages where we do not specify exacts version (such as flake8) would be different between local builds and AppVeyor, but I don't thing there is much we can do to fix this.

Any comparison logic we come up with for this is not going to play well with configobj, as it is not coming from Pypy with a specific version, but rather a git commit url. Perhaps when configobj 5.1.0 is added to Pypy we might be able to, then all of our requirements will have exact versions.

Any comparison logic we come up with for this is not going to play well with configobj, as it is not coming from Pypy with a specific version, but rather a git commit url. Perhaps when configobj 5.1.0 is added to Pypy we might be able to, then all of our requirements will have exact versions.

Well, I'd rather have logic that respects requirements not of the package==version form, including references to git commits, wheels, etc. There might be situations where we need them.

The fact that we do not have a version specified for configObj in our requirements file is not really a problem in the design I have in mint since:

• Pip handles uninstallation and installation from the URL pretty well
• The only time in which we would need a version specifier is for a packages which were installed as a dependency of no longer installed packages, so this point does not apply to configObj at all.

@lukaszgo1commented Do you think @michaelDCurran's code from #13048 (comment) should be preferred over current code?

• PASS: Translation comments check.
• PASS: Unit tests.
• PASS: Lint check.
• FAIL: System tests. See test results for more information.
• Build (for testing PR): https://ci.appveyor.com/api/buildjobs/8832fukmy5tw9f0q/artifacts/output/nvda_snapshot_pr13048-25611,a5701ff4.exe

See test results for failed build of commit a5701ff409

@seanbudd think we're ok now. I had to rebase on master because something went wrong, don't see exactly what or why.
Note that I explicitly retested the new scenario.

This breaks non-interactive (background) builds, this previously wasn't a common case because questions were only asked when the virtual env wasn't recognised as being created by NVDA, whereas updates to the requirements file are much more common.

It would be good if this took the default action (yes) after some timeout, or at least allowed --non-interactive to be specified when initiating the build.

Out of intrest, why would one build non-interactively outside of appveyor? Appveyor builds are always fresh aren't they?

I run the build in the background and output to a file. I typically do other things with the shell while it is building. I like having the build logs redirected to a file so that I can more easily inspect them after a build (if it encounters issues). It would be very unusual for me to want to avoid venv recreation.

Another situation is integration with an IDE. Most IDE's let you specify a script to run to build / clean etc.

Ugh, this is problematic indeed. I will have a closer look later today. Timeout seems to be problematic, I'd rather have a default applied per question when running non-interactively. Non-interactive mode should also be non-destructive, so answer usually should be no.

Non-interactive mode should also be non-destructive, so answer usually should be no.

I disagree here, I think the default should be the main use-case, ensuring the environment matches the requirements for this version of the source code. Wanting to skip updating the venv is the exception (special case). To take this path, you really need to know what you are doing as a developer, it is likely that the code depends on the updated requirement.

Currently the justification in this PR description is:

In many cases, this behavior is slightly harsh.

I don't think I follow the problem. Could you expand on where this causes a problem? It would be good to capture the use-case in the description.

Timeout seems to be problematic, I'd rather have a default applied per question when running non-interactively.

Yes, I'm ok with this, specifying some parameter along with scons.bat, runnvda.bat, runlint.bat rununittests.bat is fine.

One way to detect this situation is to use sys.stdout.isatty(). I don't know whether it is accurate in all cases, but it does seem to work in my use-case.

In that case, let's go with that. I'll try to come with a pr tomorrow or next week.

Thanks for the quick solution @LeonarddeR. I did some tests yesterday to understand the initial motivation for this PR. I think it is worth summing up here and adding to the description. First I'd like to check if you agree with my understanding:

• When recreating the venv, it is deleted and built from scratch.
• The alternative is to update pip and install directly from the requirements file
  • Essentially runs pip install --upgrade pip then pip install -r requirements.txt
  • This won't remove packages that are no longer needed. This may cause instability in NVDA. It will also cause unnecessary packages to be included when building the launcher.
• Recreating the venv from scratch takes much longer then just updating. It would be good to have at least one timing for this so we know the order of magnitude.

I was under the impression that pip caches the packages locally after the first install. Subsequent venv creations should be just a matter of copying the files again. I would have hoped there wasn't such a difference in the two approaches, however testing it locally it does appear to take significantly longer.