We require contributors to sign our Contributor License Agreement and we don't have one on file for @DerThorsten.

In order for us to review and merge your code, please e-sign the Contributor License Agreement PDF. We then need to manually verify your signature, merge the PR (conda/infrastructure#955), and ping the bot to refresh the PR.

@conda-bot check

CodSpeed Performance Report

Merging #13962 will not alter performance

_{Comparing DerThorsten:emscripten_platform (89aab35) with main (2f3fda3)}

Summary

✅ 21 untouched benchmarks

A few things that need to be done:

• the Arch enum also needs a wasm32 addition

Probably

• the Platform enum also needs a wasi platform

Probably, but unrelated to this PR, but I can also add it

• we need to add a news file that mentions that this is a bugfix for Add emscripten-wasm32 and wasi-wasm32 platforms to known platforms #13095
  I can do that

I can do that

Additionally, I worry that this wasn't caught earlier, could we add some tests?

I don't know enough about the testing in conda, but one could just install a package from the emscripten-forge channel

@DerThorsten,

I believe we want to avoid any integration tests especially those reaching to outside services. I'm guessing the appropriate type of test would be some form of unit test.

@jezdez,

I was looking around the test code today and couldn't really find any good places to stick these tests. We could for example create a new tests/models/test_enums.py file and put some very straightforward tests there (e.g. just make sure all the enums we want to have defined are defined). Is that the type of test you had in mind? Or were you thinking of doing something a little more complex?

two questions:

• Do the other Arch and Platform enums have tests?
• What is the value of enum unit tests here?

the thing that really matters is: will the env creation commands listed here really work after merging this PR or are we still missing something? Only integration tests or a manual test can ensure this now (and for future changes). If we don't want tests that rely on outside services, then the PR is complete? or we write an complicated test that monkey patches the enums with artificial random arch/platform and mocks a channel+platform to prove that extending the arch + platform + subdir is enough for any additions (this would be imho unfair to request from the PR author as this goes beyond fixing PR #13095)

a canary build of the PR would be nice to install conda locally and run some of the commands here https://emscripten-forge.readthedocs.io/en/latest/src/usage.html

a canary build of the PR would be nice to install conda locally and run some of the commands here https://emscripten-forge.readthedocs.io/en/latest/src/usage.html

FYI the docs moved to https://emscripten-forge.org/

If we don't want tests that rely on outside services, then the PR is complete?

So from my side, I believe its complete.

Thanks for your work on this @DerThorsten! Looking forward to this

@jezdez whats the way to move forward here?

pre-commit.ci autofix

@kenodegard Do you have an idea why we're not seeing the error output from subprocess_call_with_clean_env in the failing test?

@jezdez when an error or non-zero exit occurs the stdout & stderr are formatted and logged to INFO, unfortunately the default logging in conda is WARNING so the messages are silenced

Hm the test failure is complaining about emscripten not being a valid platform

edit: struggling to replicate the failure locally

I think the pkgs cache (which is shared across CI runs) got polluted with the test files, and now the vanilla conda (outside this PR) cannot process its own PackageCacheRecord. Let's see if clearing it up helps.

I manually cleared integration-1 caches triggered by refs/pull/13962/merge from https://github.com/conda/conda/actions/caches, and added a temporary pkgs_dirs workaround so they don't get cached again. Hopefully that's enough.

yay! 🎉