MAINT: Use bundle_tools_3 and fix various bugs #195

larsoner · 2023-07-13T13:37:02Z

A naive attempt at conda/constructor#474 (comment)

Closes #204
Closes #191
Closes #190
Closes #182
Closes #181
Closes #131

jaimergp · 2023-07-13T13:44:17Z

Btw I am uploading a new constructor beta so re-run the CI in ~30min to 1h to use it. It contains a fix for permission escalation.

larsoner · 2023-07-13T14:49:00Z

@jaimergp can you remind me how to make the step more verbose so we can see why Windows fails as the NSIS step?

https://github.com/mne-tools/mne-installers/actions/runs/5543849246/jobs/10120455858?pr=195#step:8:605

larsoner · 2023-07-13T17:09:35Z

No dice with constructor -vvv --debug, still got something not very informative:

INFO:constructor.winexe:Calling: ['C:\\Users\\runneradmin\\micromamba-root\\envs\\constructor-env\\NSIS\\makensis.exe', '/V4', 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpa7np722l\\main.nsi']
Traceback (most recent call last):
  File "C:\Users\runneradmin\micromamba-root\envs\constructor-env\Scripts\constructor-script.py", line 10, in <module>
    sys.exit(main())
  File "C:\Users\runneradmin\micromamba-root\envs\constructor-env\lib\site-packages\constructor\main.py", line 361, in main
    main_build(dir_path, output_dir=out_dir, platform=args.platform,
  File "C:\Users\runneradmin\micromamba-root\envs\constructor-env\lib\site-packages\constructor\main.py", line 185, in main_build
    create(info, verbose=verbose)
  File "C:\Users\runneradmin\micromamba-root\envs\constructor-env\lib\site-packages\constructor\winexe.py", line 429, in create
    process.check_returncode()
  File "C:\Users\runneradmin\micromamba-root\envs\constructor-env\lib\subprocess.py", line 457, in check_returncode
    raise CalledProcessError(self.returncode, self.args, self.stdout,
subprocess.CalledProcessError: Command '['C:\\Users\\runneradmin\\micromamba-root\\envs\\constructor-env\\NSIS\\makensis.exe', '/V4', 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpa7np722l\\main.nsi']' returned non-zero exit status 1.

jaimergp · 2023-07-13T19:46:37Z

I ran it locally and got this:

Name: "MNE-Python 1.4.2_0 (64-bit)"
OutFile: "C:\Users\JaimeRodriguez-Guerr\Downloads\mne-installers-bt3\mne-installers-bt3\MNE-Python-1.4.2_0-Windows.exe"
ShowInstDetails: hide
ShowUninstDetails: hide
SetCompress: off
VIAddVersionKey: "ProductName" "MNE-Python 1.4.2_0 (64-bit)"
VIAddVersionKey: "FileVersion" "1.4.2_0"
VIAddVersionKey: "ProductVersion" "1.4.2_0"
VIAddVersionKey: "CompanyName" "MNE-Python Developers"
VIAddVersionKey: "LegalCopyright" "(c) MNE-Python Developers"
VIAddVersionKey: "FileDescription" "MNE-Python Installer"
VIAddVersionKey: "Comments" "Created by constructor 0+unknown"
BrandingText: "MNE-Python Developers"
!define: "MUI_ICON"="C:\Users\JAIMER~1\AppData\Local\Temp\tmpt2a8qp5w\icon.ico"
!define: "MUI_UNICON"="C:\Users\JAIMER~1\AppData\Local\Temp\tmpt2a8qp5w\icon.ico"
!define: "MUI_HEADERIMAGE"=""
!define: "MUI_HEADERIMAGE_BITMAP"="C:\Users\JAIMER~1\AppData\Local\Temp\tmpt2a8qp5w\header.bmp"
!define: "MUI_HEADERIMAGE_UNBITMAP"="C:\Users\JAIMER~1\AppData\Local\Temp\tmpt2a8qp5w\header.bmp"
!define: "MUI_ABORTWARNING"=""
!define: "MUI_FINISHPAGE_NOAUTOCLOSE"=""
!define: "MUI_UNFINISHPAGE_NOAUTOCLOSE"=""
!define: "MUI_WELCOMEFINISHPAGE_BITMAP"="C:\Users\JAIMER~1\AppData\Local\Temp\tmpt2a8qp5w\welcome.bmp"
!define: "MUI_UNWELCOMEFINISHPAGE_BITMAP"="C:\Users\JAIMER~1\AppData\Local\Temp\tmpt2a8qp5w\welcome.bmp"
!define: "MUI_PAGE_CUSTOMFUNCTION_PRE"="SkipPageIfUACInnerInstance"
!insertmacro: MUI_PAGE_WELCOME
!insertmacro: end of MUI_PAGE_WELCOME
!define: "MUI_PAGE_CUSTOMFUNCTION_PRE"="SkipPageIfUACInnerInstance"
!insertmacro: MUI_PAGE_LICENSE
!insertmacro: end of MUI_PAGE_LICENSE
Page: Custom (creator:InstModePage_Create) (leave:InstModePage_Leave)
!define: "MUI_PAGE_CUSTOMFUNCTION_PRE"="DisableBackButtonIfUACInnerInstance"
!define: "MUI_PAGE_CUSTOMFUNCTION_LEAVE"="OnDirectoryLeave"
!insertmacro: MUI_PAGE_DIRECTORY
!insertmacro: end of MUI_PAGE_DIRECTORY
Page: Custom (creator:mui_AnaCustomOptions_Show)
!insertmacro: MUI_PAGE_INSTFILES
!insertmacro: end of MUI_PAGE_INSTFILES
'
DEBUG:constructor.winexe:makensis stderr:
'Invalid command: "{\rtf1\ansi\ansicpg1252\cocoartf2636"
Error in script "C:\Users\JAIMER~1\AppData\Local\Temp\tmpt2a8qp5w\main.nsi" on line 148 -- aborting creation process
'
Traceback (most recent call last):
  File "C:\Users\JaimeRodriguez-Guerr\devel\conda\devenv\Windows\envs\devenv-3.8-c\envs\constructor-dev\lib\runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "C:\Users\JaimeRodriguez-Guerr\devel\conda\devenv\Windows\envs\devenv-3.8-c\envs\constructor-dev\lib\runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "C:\Users\JaimeRodriguez-Guerr\devel\conda\devenv\Windows\envs\devenv-3.8-c\envs\constructor-dev\Scripts\constructor.exe\__main__.py", line 7, in <module>
  File "C:\Users\JaimeRodriguez-Guerr\devel\constructor\constructor\main.py", line 351, in main
    main_build(dir_path, output_dir=out_dir, platform=args.platform,
  File "C:\Users\JaimeRodriguez-Guerr\devel\constructor\constructor\main.py", line 175, in main_build
    create(info, verbose=verbose)
  File "C:\Users\JaimeRodriguez-Guerr\devel\constructor\constructor\winexe.py", line 439, in create
    process.check_returncode()
  File "C:\Users\JaimeRodriguez-Guerr\devel\conda\devenv\Windows\envs\devenv-3.8-c\envs\constructor-dev\lib\subprocess.py", line 457, in check_returncode
    raise CalledProcessError(self.returncode, self.args, self.stdout,
subprocess.CalledProcessError: Command '['C:\\Users\\JaimeRodriguez-Guerr\\devel\\conda\\devenv\\Windows\\envs\\devenv-3.8-c\\envs\\constructor-dev\\NSIS\\makensis.exe', '/V4', 'C:\\Users\\JAIMER~1\\AppData\\Local\\Temp\\tmpt2a8qp5w\\main.nsi']' returned non-zero exit status 1.

It looks like it doesn't like the RTF files for some weird reason? 🤔

larsoner · 2023-07-13T19:49:44Z

Awesome -- maybe/probably because there is unicode... I'll try opening it in Windows and re-saving it. If that doesn't work I'll remove the unicode and try again!

larsoner · 2023-07-13T19:52:46Z

None of the unicode rendered well in Windows except for one folder icon, so I just deleted the unicode business and re-saved them. Looks like all files got smaller. 🤞

This reverts commit f378242.

jaimergp · 2023-07-13T19:56:30Z

Mm, no I think it's a bug introduced in this release (probably this line). conclusion_file can also be a NSI script, and in that case it gets inlined. I can see locally that RTF contents were dumped directly in the script.

I think the only workaround for now is to disable the custom conclusion file on Windows installers. I'll open a PR on constructor next week.

Can you open an issue to track this in conda/constructor? Thanks 🙏

hoechenberger · 2023-07-13T20:31:18Z

@larsoner Can you revert the change to the RTF file please?

larsoner · 2023-07-13T20:40:47Z

Have you checked to see if they render on Windows? Only one worked when I loaded it in WordPad and Word for me, all other Unicode characters rendered as weird vertical lines. f you haven't checked I will, and if they don't show on Windows we can either have two sets of files or (my preference) live with not having Unicode anywhere...

larsoner · 2023-07-13T21:24:09Z

@jaimergp looks like that was the bug on Windows! For macOS now we get an error on installation:

2023-07-13 20:39:37+00 Mac-1689278542280 installer[2912]: install:didFailWithError:Error Domain=PKInstallErrorDomain Code=501 "The package is attempting to install content to the system volume." UserInfo={NSLocalizedDescription=The package is attempting to install content to the system volume.}

and on Linux we have a post-install script meant to fix the .desktop files but it looks like they are no longer produced in $HOME/.local/share/applications:

ℹ️ Fixing menu shortcuts.

sed: can't read ./MNE-Python*.desktop: No such file or directory
ERROR: executing post_install.sh failed

Any thoughts on these?

hoechenberger · 2023-07-14T10:45:18Z

@larsoner We're applying a number of patches, maybe they need modification…
For example:
https://github.com/mne-tools/mne-installers/blob/main/assets/constructor_macOS_common.patch

Edit: Then again, this is just for post-install…

larsoner · 2023-07-14T15:07:57Z

@larsoner We're applying a number of patches,...
Edit: Then again, this is just for post-install…

Actually I don't see these used anywhere other than /tools/build_local.sh, which is not used by CIs. Even worse, build_local.sh references a patch_constructor.sh file that doesn't exist. So I don't think these patches are even used anymore!

$ git grep patch
.github/workflows/build.yml:  workflow_dispatch:
.github/workflows/build.yml:            # … and patched to work around a bug in menuinst
README.md:3. Run `./tools/build_local.sh` (which will patch the constructor on macOS if needed).
assets/constructor_macOS_arm64.patch:diff -Naur constructor-orig/osxpkg.py constructor-patched/osxpkg.py
assets/constructor_macOS_arm64.patch:+++ constructor-patched/osxpkg-patched.py  2022-06-02 23:31:41.000000000 +0200
assets/constructor_macOS_common.patch:diff -Naur constructor-orig/osx/post_extract.sh constructor-patched/osx/post_extract.sh
assets/constructor_macOS_common.patch:+++ constructor-patched/osx/post_extract.sh       2022-03-19 20:28:13.000000000 +0100
assets/constructor_macOS_common.patch:diff -Naur constructor-orig/osxpkg.py constructor-patched/osxpkg.py
assets/constructor_macOS_common.patch:+++ constructor-patched/osxpkg.py 2022-03-19 20:15:12.000000000 +0100
assets/constructor_macOS_i386.patch:diff -Naur constructor-orig/osxpkg.py constructor-patched/osxpkg.py
assets/constructor_macOS_i386.patch:+++ constructor-patched/osxpkg-patched.py   2022-06-02 23:31:41.000000000 +0200
tools/build_local.sh:${SCRIPT_DIR}/patch_constructor.sh

Notably the patch_constructor.sh does not even exist anymore, so we should probably just remove some or all of that stuff as it appears to be cruft (or reintegrate it properly, which you would probably have to do @hoechenberger ).

So the macOS install failure seems new and relevant with bundle_tools_3, and the Linux .desktop files missing also seems relevant @jaimergp !

jaimergp · 2023-07-17T08:14:12Z

I would list the contents of the directory. They should be there, but maybe I think the names are now "slugified" so it'd be all lowercase, no spaces etc.

Also, I will fix the .desktop True/true problem on menuinst. Let me know if you find something else!

assets/constructor_macOS_common.patch

jaimergp · 2023-07-17T16:02:18Z

Also, I will fix the .desktop True/true problem on menuinst. Let me know if you find something else!

Turns out this had been done a while ago too.

recipes/mne-python_1.4/construct.yaml

assets/constructor_macOS_common.patch

assets/constructor_macOS_arm64.patch

assets/constructor_macOS_i386.patch

larsoner · 2023-09-01T18:21:17Z

FYI I set the data retention period to 5 days for this repo directly in main. I then removed the purge-artifacts action that ran daily since I don't think it's necessary anymore -- the repo-level actions retention applies to both CI runs and artifacts (and had already been set at 1 day) so I don't think it's needed anymore -- I think it came from before the artifacts were part of the retention period.

Hopefully the latest commit I pushed is the last one I'll need, and I'll update the URL above to have the latest artifact link once it finishes!

larsoner · 2023-09-01T19:45:03Z

assets/Menu/mne_open_prompt.bat

+:: This is used to initialize the bash prompt on Windows.
+@ECHO OFF
+
+call %/home/conda/feedstock_root/build_artifacts/mne-python_1692127656949/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_%\Scripts\Activate.bat


This makes me think that the hack of doing menuinst ourselves will not work. Either menuinst or constructor must do something magical here. I'll revert for now but c5dfc2a seems close to what we want

This reverts commit c5dfc2a.

assets/post_install_macOS.sh

Co-authored-by: Richard Höchenberger <[email protected]>

cbrnr · 2023-09-02T07:36:17Z

I am still debugging Windows a bit but the macOS installers seem to work locally for me on M1 in both single-user mode (but only puts it in ~/Applications, which I guess is the best we can do) and system-wide mode

Quick question, what is the difference between single-user and system-wide modes? Many apps install into ~/Applications when installed as non-root.

Do we mention what exactly happens when running our installers? I'm always concerned with installers modifying stuff that cannot be easily reverted, so if we describe what we're actually doing (e.g. creating which folder) for both modes that would be a great way for people like me to try out our installers.

larsoner · 2023-09-02T11:54:10Z

It would be a good idea to add a note about that to the installers page of the MNE-Python docs after we merge this, especially since on Linux and macOS there is no uninstaller so you have to remove things manually

larsoner · 2023-09-02T15:08:30Z

... actually I don't like how macOS installs stuff to /Library and /Applications currently. Most applications I have just live in /Applications and if I want them gone I drag them to the trash. I'm going to install to /Applications/MNE-Python/1.5.0_1 instead, which has multiple advantages:

allows people to multiple versions installed safely
is consistent with other platforms
allows the Python distribution to live in /Applications/MNE-Python/1.5.0_1/.mne-python, which is hidden but therefore...
...allows them to uninstall everything just by dragging the 1.5.0_1 or MNE-Python directories to the trash

Then macOS and Windows both uninstall the way users would expect (I think), and Linux is the only annoying one (.desktop files need to be manually removed). I'll push a commit shortly...

larsoner · 2023-09-02T17:05:41Z

Latest artifacts:

https://github.com/mne-tools/mne-installers/actions/runs/6059299682?pr=195#artifacts

jaimergp · 2023-09-03T10:15:32Z

The choice for Library vs Applications was that some users expect things under Applications to be self-contained relocatable, which is not always the case in conda-based installations.

cbrnr · 2023-09-03T13:10:52Z

The choice for Library vs Applications was that some users expect things under Applications to be self-contained relocatable, which is not always the case in conda-based installations.

But with the changes @larsoner made in this PR, this is now the case, or no? I like that everything is contained in a single folder, which makes uninstalling everything a trivial task.

larsoner · 2023-09-03T15:15:14Z

But with the changes @larsoner made in this PR, this is now the case, or no?

I now put everything in Applications, which has the advantage of easy uninstallation but will break if people move things around.

cbrnr · 2023-09-03T18:13:27Z

Move what around? There's just one folder - it breaks when they move it? If that's the issue, we can document this behavior and tell people to uninstall and then reinstall in the desired location. Or point out that the folder cannot be moved, which is OK, because it is in the expected standard location.

Can you explain the difference between system-wide and user installation?

larsoner · 2023-09-03T18:27:11Z

If you try the installer it will probably become clear. But by single user I mean installing to ~/Applications and by system wide I mean to /Applications

larsoner · 2023-09-06T01:14:10Z

Okay I added some basic set of dev dependencies (I couldn't even run pytest with the previous version!) and new artifacts are here:

https://github.com/mne-tools/mne-installers/actions/runs/6091167620?pr=195#artifacts

Unless anyone objects I'll merge tomorrow to keep things moving and we can deal with other stuff in follow-up PRs.

larsoner added 2 commits July 13, 2023 09:36

MAINT: Use bundle_tools_3

48ae61d

FIX: No patch

2930c71

WIP: Maybe?

0678ef9

FIX: Wordpad

38f5148

larsoner added 2 commits July 13, 2023 15:55

FIX: Again

f378242

Revert "FIX: Again"

7ad989e

This reverts commit f378242.

FIX: NOT WIN

b74eeee

larsoner mentioned this pull request Jul 13, 2023

Bug when using custom conclusion_file on Windows conda/constructor#700

Closed

larsoner added 2 commits July 17, 2023 11:00

MAINT: Rerun and cruft

4938ecd

FIX: No patch

b959606

larsoner commented Jul 17, 2023

View reviewed changes

assets/constructor_macOS_common.patch Show resolved Hide resolved

larsoner added 2 commits July 17, 2023 12:16

FIX: Defaults

ca3fa25

FIX: SPacing

1afb4b3

jaimergp reviewed Jul 17, 2023

View reviewed changes

recipes/mne-python_1.4/construct.yaml Outdated Show resolved Hide resolved

larsoner commented Jul 17, 2023

View reviewed changes

assets/constructor_macOS_common.patch Show resolved Hide resolved

assets/constructor_macOS_arm64.patch Show resolved Hide resolved

assets/constructor_macOS_i386.patch Show resolved Hide resolved

FIX: Suggestion

9fd2a61

larsoner added 3 commits September 1, 2023 13:31

FIX: Facilitate editable install

49223a5

FIX: Msg

e595c16

FIX: Remove timeout

483788a

larsoner changed the title ~~MAINT: Use bundle_tools_3~~ MAINT: Use bundle_tools_3 and fix various bugs Sep 1, 2023

This was referenced Sep 1, 2023

BUG: macOS and Windows minor misc issues #90

Closed

Move installer menus from mne-feedstock to this repo #100

Closed

ENH: Menus here

c5dfc2a

larsoner commented Sep 1, 2023

View reviewed changes

Revert "ENH: Menus here"

ea00c68

This reverts commit c5dfc2a.

hoechenberger reviewed Sep 1, 2023

View reviewed changes

assets/post_install_macOS.sh Outdated Show resolved Hide resolved

larsoner and others added 2 commits September 1, 2023 18:26

Update assets/post_install_macOS.sh [ci skip]

961f550

Co-authored-by: Richard Höchenberger <[email protected]>

Merge branch 'main' into bt3

b847b41

larsoner added 2 commits September 2, 2023 11:15

FIX: Nest

7f10d03

FIX: Whoops

d9d2415

larsoner added 2 commits September 5, 2023 14:07

ENH: Add development tools

ee224f6

FIX: Versions

5585a04

larsoner merged commit 98d7873 into mne-tools:main Sep 6, 2023

larsoner deleted the bt3 branch September 6, 2023 17:51

larsoner mentioned this pull request Sep 6, 2023

MAINT: Bump to 1.5.1 #205

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MAINT: Use bundle_tools_3 and fix various bugs #195

MAINT: Use bundle_tools_3 and fix various bugs #195

larsoner commented Jul 13, 2023 •

edited

Loading

jaimergp commented Jul 13, 2023

larsoner commented Jul 13, 2023

larsoner commented Jul 13, 2023

jaimergp commented Jul 13, 2023

larsoner commented Jul 13, 2023

larsoner commented Jul 13, 2023

jaimergp commented Jul 13, 2023

hoechenberger commented Jul 13, 2023

larsoner commented Jul 13, 2023

larsoner commented Jul 13, 2023

hoechenberger commented Jul 14, 2023 •

edited

Loading

larsoner commented Jul 14, 2023

jaimergp commented Jul 17, 2023

jaimergp commented Jul 17, 2023

larsoner commented Sep 1, 2023 •

edited

Loading

larsoner Sep 1, 2023

cbrnr commented Sep 2, 2023

larsoner commented Sep 2, 2023

larsoner commented Sep 2, 2023 •

edited

Loading

larsoner commented Sep 2, 2023

jaimergp commented Sep 3, 2023

cbrnr commented Sep 3, 2023

larsoner commented Sep 3, 2023

cbrnr commented Sep 3, 2023

larsoner commented Sep 3, 2023

larsoner commented Sep 6, 2023

MAINT: Use bundle_tools_3 and fix various bugs #195

MAINT: Use bundle_tools_3 and fix various bugs #195

Conversation

larsoner commented Jul 13, 2023 • edited Loading

jaimergp commented Jul 13, 2023

larsoner commented Jul 13, 2023

larsoner commented Jul 13, 2023

jaimergp commented Jul 13, 2023

larsoner commented Jul 13, 2023

larsoner commented Jul 13, 2023

jaimergp commented Jul 13, 2023

hoechenberger commented Jul 13, 2023

larsoner commented Jul 13, 2023

larsoner commented Jul 13, 2023

hoechenberger commented Jul 14, 2023 • edited Loading

larsoner commented Jul 14, 2023

jaimergp commented Jul 17, 2023

jaimergp commented Jul 17, 2023

larsoner commented Sep 1, 2023 • edited Loading

larsoner Sep 1, 2023

Choose a reason for hiding this comment

cbrnr commented Sep 2, 2023

larsoner commented Sep 2, 2023

larsoner commented Sep 2, 2023 • edited Loading

larsoner commented Sep 2, 2023

jaimergp commented Sep 3, 2023

cbrnr commented Sep 3, 2023

larsoner commented Sep 3, 2023

cbrnr commented Sep 3, 2023

larsoner commented Sep 3, 2023

larsoner commented Sep 6, 2023

larsoner commented Jul 13, 2023 •

edited

Loading

hoechenberger commented Jul 14, 2023 •

edited

Loading

larsoner commented Sep 1, 2023 •

edited

Loading

larsoner commented Sep 2, 2023 •

edited

Loading