search

pytorch / test-infra

This repository hosts code that supports the testing infrastructure for the PyTorch organization. For example, this repo hosts the logic to track disabled tests and slow tests, as well as our continuation integration jobs HUD/dashboard.
https://hud.pytorch.org/
Other
82 stars 79 forks source link

Do we need our custom setup-miniconda action? #3843

Open pmeier opened 1 year ago

pmeier commented 1 year ago

The generic macOS job uses our custom action to setup conda:

https://github.com/pytorch/test-infra/blob/d70ae5780cb5c81a6eb706e305104825cf42f454/.github/workflows/macos_job.yml#L84-L85

However, this is limited to macOS and Linux:

https://github.com/pytorch/test-infra/blob/d70ae5780cb5c81a6eb706e305104825cf42f454/.github/actions/setup-miniconda/README.md?plain=1#L5-L9

Although https://github.com/pytorch/test-infra/pull/614#discussion_r957624393 implies that adding other platforms is as easy as adding more cases to the case statement, this is unfortunately not true for Windows. Anaconda only provides an .exe there which you cannot invoke from bash directly. Of course I can do something like cmd.exe /c miniconda.exe, but then I have to mess with the paths, since cmd doesn't understand something like /miniconda. TL;DR I couldn't get it to work.

Today I noticed we are using

https://github.com/pytorch/test-infra/blob/d70ae5780cb5c81a6eb706e305104825cf42f454/.github/actions/setup-binary-builds/action.yml#L92

for the binary build setup. If that is ok, do we even need to roll our custom solution in the first place? conda-incubator/setup-miniconda has seamless support for Windows and seemed to even be the basis of our custom action:

https://github.com/pytorch/test-infra/blob/d70ae5780cb5c81a6eb706e305104825cf42f454/.github/actions/setup-miniconda/action.yml#L30-L31

grep'ing through the repository, it seems we aren't using it for anything but the generic macOS job.

osalpekar commented 1 year ago

cc @seemethere might have more context here

seemethere commented 1 year ago

The setup-miniconda action didn't work for us when I was trying it without having the need to have special shell setups.

We also wanted to have a bit more control on what the action was doing. In my mind as well the less dependencies we have on third party actions the better since those third party dependencies can introduce a lot of ambiguity in our CI workflows.

pmeier commented 1 year ago

Meaning we should actually remove the third-party action for our own one in the binary build setup? Any idea how we can support Windows in such a scenario? It seems it hasn't received much love so far, but will have to at some point to migrate away from CircleCI.

seemethere commented 1 year ago

I believe @atalman will eventually do this one when we officially add Windows support