Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pyinstaller part 2 #2862

Merged
merged 11 commits into from
Jul 19, 2024
Merged

Pyinstaller part 2 #2862

merged 11 commits into from
Jul 19, 2024

Conversation

krzywon
Copy link
Contributor

@krzywon krzywon commented Apr 26, 2024

Description

This is a tweak to how pyinstaller creates packages. Due to a couple of changes on the Mac build, this also adds some fixes to the documentation viewer to ensure Mac docs are copied into the user folder when required and are displayed properly.

Fixes #2886
Fixes #2813
Fixes #2902

Review Checklist:

[if using the editor, use [x] in place of [ ] to check a box]

Documentation (check at least one)

  • There is nothing that needs documenting
  • Documentation changes are in this PR
  • There is an issue open for the documentation (link?)

Installers

  • This will affect the installers
    • Windows installer (GH artifact) has been tested (installed and worked)
    • MacOSX installer (GH artifact) has been tested (installed and worked)
    • Ubuntu installer (GH artifact) has been tested (installed and worked)

Licencing (untick if necessary)

  • The introduced changes comply with SasView license (BSD 3-Clause)

@klytje
Copy link
Contributor

klytje commented Apr 26, 2024

sasview now succesfully opens, but crashes the moment I click the 'open file' pane (both in the GSC and Image Viewer; so I assume it's all of them) with the error:
image

@rozyczko
Copy link
Member

On windows, this build seems to run fine.

@butlerpd butlerpd added the SasView 6.0.0 Required for 6.0.0 release label Apr 29, 2024
@krzywon
Copy link
Contributor Author

krzywon commented Apr 30, 2024

sasview now succesfully opens, but crashes the moment I click the 'open file' pane (both in the GSC and Image Viewer; so I assume it's all of them) with the error: image

@klytje - Thanks for the report. Are you seeing this error with anything other programs? It looks like either a permissions or environment variable issue. I'll see what I can do about this from our end.

https://askubuntu.com/questions/1351607/gtk-warning-could-not-load-a-pixbuf-from-icon-theme
https://bbs.archlinux.org/viewtopic.php?id=261143

@klytje
Copy link
Contributor

klytje commented May 1, 2024

@krzywon I've never seen this error before (and I have both programs mentioned in that link installed). I am developing a GUI program myself relying on GTK, so I'd be very surprised if that's the issue. Anyway, I've now tested it on all of my systems.

The same issue is present on 2 separate Ubuntu 22.04 systems. On my Ubuntu 20.04 VM it works just fine. We need more test systems to know for sure if it's just my systems or if it could be 22.04 in general.

Errors unique to 22.04:
image

I'm not sure if it is related to your PR though - this is the first time I've tried running an installer from the release_6.0.0 branch without having it immediately crash on me.

@llimeht
Copy link
Contributor

llimeht commented May 23, 2024

If the SasView windows installer is used to do a system-wide installation (which it can do and I think that's even the default), then it's buggy to be using {%USERPROFILE} in the installer config (.iss). The problem in the context of a system-wide installation is which single user will get the files in their profile, and what about other users? What happens with a central IT person insists on installing the software for the user, or SasView is installed on shared computers at a facility to be used by lots of different users?

There is an existing use of {%USERPROFILE} - but that's an empty directory that (I think) gets recreated at runtime if needed. (We should make sure that is true and then get rid of that entry in the .iss.)

To run sphinx at runtime in a user-writeable a location, some other creative solution is needed to correctly handle multi-user systems. Lazily copying the documentation across to the user profile on first start might work. Even better would be copying the documentation only once the user asks to edit it; rehoming the doc-base from the installer-provided files to the user-edited files only when needed would also address #2886.

Some work also needs to go into preventing data loss where the installer-provided documentation overwrites local edits to those files when upgrading versions. I think that would currently occur with the approach in this PR. (The above suggestions of lazily copying the files around could have similar issues depending on how they are implemented. Syncing is a hard problem.)

Copy link
Member

@butlerpd butlerpd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing functionality on Windows only, this does fix the problem identified in #2886. My approval is solely for that part of this PR. Others should test the mac and linux issues.

Copy link
Contributor

@tsole0 tsole0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes work on Mac and fixes #2813. My approval is specific to the Mac-dependent part of the PR.

@krzywon krzywon merged commit 1ffe82e into release_6.0.0 Jul 19, 2024
29 checks passed
@krzywon krzywon deleted the pyinstaller-part-deux branch July 19, 2024 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SasView 6.0.0 Required for 6.0.0 release
Projects
None yet
7 participants