-
Notifications
You must be signed in to change notification settings - Fork 175
Improve logging for EXE installers #1108
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
Draft
marcoesters
wants to merge
23
commits into
conda:main
Choose a base branch
from
marcoesters:add-logger
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
3 tasks
Contributor
Author
|
@jaimergp, this can get a preliminary review, but I will keep it in draft until conda/conda-standalone#218 is merged. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The logging experience for EXE installers has several problems that make debugging difficult to impossible:
install.logfiles to appear inside%USERPROFILE%or even cause the installation to fail because the check for empty directories fail due to the log file.icacls.exeorpython.exe) fails.This PR addresses these issues by:
$INSTDIRhas been created.cmd.exesubshell to output error messages coming from executing the binary (e.g., if it's not found or if there are permission issues). This makespythonw.exeobsolete and outputs errors from the custom Python script.AbortRetryNSExecWaitso that logs are still available if the uninstallation fails (continuing even though steps fail shouldn't be happening in the first place to avoid undefined behavior, in my opinion).I also decided to remove unused functions, which resulted in
Utils.nshonly containing theIsWritablefunction. I could move that function into the main template, or I can move the functions inside theFunctionTemplatesmacro and thePrintmacro intoUtils.nsh.Caveats:
condacommands cannot directly write intoinstall.logeven if they support--log-filebecause NSIS maintains the file handle on the log file and doesn't allow writing to it outsideLogSet.cmd.exedoes not support theteecommand, there is a trade-off between live printing for GUI installers and logging for system calls. I prioritized getting real time output into the GUI window over writing to the log file.This also closes #998
Xref: conda/conda-standalone#218
Checklist - did you ...
newsdirectory (using the template) for the next release's release notes?