search

mne-tools / mne-installers

Installers for MNE-Python.
BSD 3-Clause "New" or "Revised" License
9 stars 8 forks source link

WIP: Bash entry for windows, normalize names #113

Closed larsoner closed 2 years ago

larsoner commented 2 years ago
  1. it seems like we should use _post2 rather than _2 in our names as it's more standard (and it's what we're calling our releases anyway)
  2. This uses the _2 conda builds, so it should include a Bash entry on Windows now.

@hoechenberger if you're happy with this, could you merge and cut a release draft, then you or I could test on Windows? I'd encourage you to try VirtualBox again, the ~20 GB Windows test VM worked pretty well for me there in testing!

hoechenberger commented 2 years ago

No, not happy. The only reason I append _number is because _YYYYMMDD got too long when I tried it early on. But I think we have since changed the string before that and we can try and see if adding the date again would work.

I don't want to bother users with highly technical stuff like postwhatever, while still indicating which revision the installers have.

I want some sort of counter, or the date (which I'd prefer)

larsoner commented 2 years ago

I want some sort of counter, or the date (which I'd prefer)

I think a counter might be better, for the same reasons as above: 1) users probably don't care about this technicality, and 2) it makes the name even longer. I'll revert to _2

larsoner commented 2 years ago

Hooray, a conflict on Windows :(

The other unsolved issue here is why I had to -eq 3 here for .desktop and .app rather than -eq 5 on main. https://github.com/mne-tools/mne-installers/pull/114 should tell us if it's really from this PR or from (not) checking the exit codes of all commands.

larsoner commented 2 years ago

https://github.com/mne-tools/mne-installers/pull/114 should be merged first, then I'll rebase this

larsoner commented 2 years ago

Wow, looks like adding bash ended up breaking windows. It can't resolve the deps.

@hoechenberger I think we should revert https://github.com/conda-forge/mne-feedstock/pull/89 and https://github.com/conda-forge/mne-feedstock/pull/90 (by way of actually increasing the build number), WDYT?

It would have been nice to know that adding bash somehow made things impossible to resolve. I guess that's a problem where mne-feedstock does not build the mne-installer-menus target. @hoechenberger do you know a way to add a test for it? In other words, we want a way to see this problem when trying to make a mne-installer-menus run env, which must not happen in mne-feedstock.

hoechenberger commented 2 years ago

I guess that's a problem where mne-feedstock does not build the mne-installer-menus target.

Bumping the build number should enforce a rebuild though … And in fact, the package was rebuilt: https://anaconda.org/conda-forge/mne-installer-menus/files

So I'm not sure where the conflict is coming from …

Oh, yeah, I might have an idea. The package is noarch so it's only built on Linux. We could make it non-noarch to avoid such breakage in the future, I think. But it really shouldn't happen in the first place :(

larsoner commented 2 years ago

Let's try running now, maybe it was some temporary resolution error.

If it doesn't work, we should just revert https://github.com/conda-forge/mne-feedstock/pull/89 so that we can make the 1.0.1 installers.