Add file logger #218

marcoesters · 2025-11-11T19:32:00Z

Description

Add the ability to log conda and conda-standalone output into a file. This feature essentially simulates the tee command.

This can be used to populate log files for installations, especially on systems where tee is not an option (Windows, in particular). One caveat of capturing output streams this way is that carriage returns are not properly handled and are output into new lines. This means that

Executing transaction... working... done

becomes

Executing transaction...
working...
done

I also added some more logging and error handling output to the uninstallation process.

Xref: conda/constructor#1108

Checklist - did you ...

Add a file to the news directory (using the template) for the next release's release notes?
Add / update necessary tests?
Add / update outdated documentation?

… add-logger

lrandersson · 2025-11-14T15:18:59Z

news/218-add-logger

 ### Enhancements

-* <news item>
+* Add option to log `conda-standalone` and `conda` outputs into log file. (#218)


Suggested change

* Add option to log `conda-standalone` and `conda` outputs into log file. (#218)

* Add option to log `conda-standalone` and `conda` outputs into a log file. (#218)

Optional: I think it could be good also to mention how to enable it. I'm thinking in the perspective of the user, help them know what to look for in order to enable it.

Thanks! Good call on adding the CLI option

lrandersson · 2025-11-14T15:21:57Z

src/conda_constructor/uninstall.py

+                    f"Failed to unprotect {env_prefix}. Try to re-run the uninstallation with "
+                    f"elevated privileges or remove the file {frozen_file} manually."


Should we wrap the {...} in simple quotes like '{...}'? Just thinking if this is something that could contain spaces etc.

I think that's a good idea - done

lrandersson · 2025-11-14T15:23:10Z

src/conda_constructor/uninstall.py

-        conda_main("remove", "-y", "-p", str(env_prefix), "--all")
+        return_code = conda_main("remove", "-y", "-p", str(env_prefix), "--all")
+        if return_code != 0:
+            raise RuntimeError(f"Failed to remove environment {env_prefix}.")


Not sure if this is necessary, but would help with debugging. Feel free to just ignore.

Suggested change

raise RuntimeError(f"Failed to remove environment {env_prefix}.")

raise RuntimeError(f"Failed to remove environment {env_prefix}. Return code: {return_code}")

Realistically, the return code won't tell us more than the preceding error messages, so I don't think the return code is necessary.

lrandersson · 2025-11-14T15:24:00Z

src/conda_constructor/uninstall.py

-    conda_main("clean", "--all", "-y")
+    return_code = conda_main("clean", "--all", "-y")
+    if return_code != 0:
+        logger.warning("Failed to remove all cache files.")


Same here (if we should add the return_code to the warning). Perhaps even less important since it's just a warning.

lrandersson · 2025-11-14T15:28:00Z

src/entry_point.py

+    def flush(self):
+        pass


Just curious why this is needed. Maybe a docstring could help here understand why we just implement it as empty. I assume it's not necessary to flush since we the write methods write directly via self.logger.log.

Your guess is correct. We must have a flush method implemented, but the log call does all that work already. I added a comment.

lrandersson

I know you need a secondary review but LGTM, well done!

jaimergp · 2025-11-15T17:46:42Z

src/conda_constructor/uninstall.py

+logger = logging.getLogger()
+# On Windows, these warnings are expected because the uninstaller may still be
+# accessing files (like install.log) that conda cannot rename.
+if sys.platform == "win32":
+    conda_logger = logging.getLogger("conda.gateways.disk.delete")
+    conda_logger.addFilter(lambda record: "Could not remove or rename" not in record.getMessage())


Do we need this as import-time logic, or should it be a separate function we call as part of the CLI initialization? Is that too late in the process?

I like it for the visibility, but you are right that it isn't strictly needed at import time. So, I went a third way: importing these main execution function isn't needed until we actually use it. So, I just moved the import statements into the execute function.

src/conda_constructor/uninstall.py

src/entry_point.py

jaimergp · 2025-11-15T17:50:42Z

src/entry_point.py

+    logger_parser = argparse.ArgumentParser(add_help=False)
+    logger_parser.add_argument("--log-file", type=Path)
+    args, remaining = logger_parser.parse_known_args()
+    if args.log_file:
+        sys.argv[1:] = remaining
+        setup_logger(args.log_file.resolve())
+
    return main()


If you turn setup_logger into a context manager, then change this to:

Suggested change

logger_parser = argparse.ArgumentParser(add_help=False)

logger_parser.add_argument("--log-file", type=Path)

args, remaining = logger_parser.parse_known_args()

if args.log_file:

sys.argv[1:] = remaining

setup_logger(args.log_file.resolve())

return main()

logger_parser = argparse.ArgumentParser(add_help=False)

logger_parser.add_argument("--log-file", type=Path)

args, remaining = logger_parser.parse_known_args()

if args.log_file:

sys.argv[1:] = remaining

maybe_setup_logger = setup_logger(args.log_file.resolve())

else:

maybe_setup_logger = nullcontext() # this comes from contextlib

with maybe_setup_logger:

return main()

jaimergp · 2025-11-15T17:52:09Z

tests/test_main.py

+            # on CI, setup-miniconda registers `test` as auto-activate for every CMD
+            # which adds some unnecessary stderr output; so, only read the first line
+            log_text = log_text.split("\n")[0]
+        assert os.path.realpath(log_text.strip()) == os.path.realpath(CONDA_EXE)


Do we want to assert whether the process had any output? All of it should have technically gone to the logfile, right?

Suggested change

assert os.path.realpath(log_text.strip()) == os.path.realpath(CONDA_EXE)

assert os.path.realpath(log_text.strip()) == os.path.realpath(CONDA_EXE)

assert not process.stdout + process.stderr

Output goes to both the screen and the log file, so we should still check the output.

Co-authored-by: jaimergp <[email protected]>

src/conda_constructor/uninstall.py

marcoesters added 5 commits November 4, 2025 13:05

Add option to also forward screen output to log file

fafc282

Add log file variant to conda run test

21966cb

Merge branch 'main' of https://github.com/conda/conda-standalone into…

4228f20

… add-logger

Add logging output and error handling to uninstallation

bae7ca9

Add news file

d70ede9

marcoesters mentioned this pull request Nov 11, 2025

Improve logging for EXE installers conda/constructor#1108

Merged

3 tasks

conda-bot added this to 🔎 Review Nov 11, 2025

github-project-automation bot moved this to 🆕 New in 🔎 Review Nov 11, 2025

conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Nov 11, 2025

Accommodate extra stderr in Windows tests

df6796c

marcoesters marked this pull request as ready for review November 12, 2025 00:03

marcoesters requested a review from a team as a code owner November 12, 2025 00:03

lrandersson reviewed Nov 14, 2025

View reviewed changes

marcoesters added 3 commits November 14, 2025 13:06

Add comment explaining pass in flush

3a53c13

Wrap paths in error messages in single quotes

2c48299

Mention --log-file in release notes

c3c6ff8

lrandersson approved these changes Nov 14, 2025

View reviewed changes

jaimergp reviewed Nov 15, 2025

View reviewed changes

src/conda_constructor/uninstall.py Outdated Show resolved Hide resolved

jaimergp reviewed Nov 15, 2025

View reviewed changes

src/conda_constructor/uninstall.py Outdated Show resolved Hide resolved

jaimergp reviewed Nov 15, 2025

View reviewed changes

src/conda_constructor/uninstall.py Outdated Show resolved Hide resolved

jaimergp reviewed Nov 15, 2025

View reviewed changes

src/entry_point.py Show resolved Hide resolved

jaimergp reviewed Nov 15, 2025

View reviewed changes

marcoesters and others added 5 commits November 17, 2025 07:09

Apply suggestions from code review

cf06a5c

Co-authored-by: jaimergp <[email protected]>

Merge branch 'main' into add-logger

16d82f2

Make logger setup a context manager

302fcbf

Import constructor plug-in execution functions only when needed

eb3a64b

Lint

dbb27b2

marcoesters requested a review from jaimergp November 17, 2025 17:03

danyeaw added this to conda Roadmap and Sprint Planning Nov 19, 2025

github-project-automation bot moved this to Sorting ⚙️ in conda Roadmap and Sprint Planning Nov 19, 2025

danyeaw moved this from Sorting ⚙️ to In review 🔍 in conda Roadmap and Sprint Planning Nov 19, 2025

jaimergp reviewed Nov 21, 2025

View reviewed changes

src/conda_constructor/uninstall.py Show resolved Hide resolved

Apply suggestions from code review

6b4aeba

jaimergp approved these changes Nov 21, 2025

View reviewed changes

github-project-automation bot moved this from 🆕 New to ✅ Approved in 🔎 Review Nov 21, 2025

marcoesters merged commit 546d786 into conda:main Nov 21, 2025
7 checks passed

marcoesters deleted the add-logger branch November 21, 2025 17:43

github-project-automation bot moved this from ✅ Approved to 🏁 Done in 🔎 Review Nov 21, 2025

github-project-automation bot moved this from In review 🔍 to Done 💪🏾 in conda Roadmap and Sprint Planning Nov 21, 2025

	* Add option to log `conda-standalone` and `conda` outputs into log file. (#218)
	* Add option to log `conda-standalone` and `conda` outputs into a log file. (#218)

		f"Failed to unprotect {env_prefix}. Try to re-run the uninstallation with "
		f"elevated privileges or remove the file {frozen_file} manually."

	raise RuntimeError(f"Failed to remove environment {env_prefix}.")
	raise RuntimeError(f"Failed to remove environment {env_prefix}. Return code: {return_code}")

	assert os.path.realpath(log_text.strip()) == os.path.realpath(CONDA_EXE)
	assert os.path.realpath(log_text.strip()) == os.path.realpath(CONDA_EXE)
	assert not process.stdout + process.stderr

Add file logger #218

Add file logger #218

Uh oh!

Conversation

marcoesters commented Nov 11, 2025

Description

Checklist - did you ...

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcoesters Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lrandersson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

marcoesters Nov 14, 2025 •

edited

Loading