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

Linux script changes #7

Merged
merged 2 commits into from
Jun 11, 2024
Merged

Linux script changes #7

merged 2 commits into from
Jun 11, 2024

Conversation

DarlCat
Copy link
Member

@DarlCat DarlCat commented Jun 10, 2024

I've been using these scripts in my local installs for a few months, and I think that they're ready for real review and testing by the rest of the team and the community. The features work on my system (Arch/GNOME), but I have not had the chance to test other desktop environments or distros.

These changes accomplish the following:

  1. Ports scripts to sh over bash
  2. XDG SLURL handling
  3. Viewer installs are now put into channel-defined folders, and get their own app launchers (e.g. Beta can be installed alongside a Project or Test viewer without conflict) - This is done by parsing the build_data.json file generated by the build process. The installer/wrapper going forward will always rely on this bundled file for pathing, window class association, and launcher generation.

Credit

This pull request contains work by @Jenna-Huntsman and @XenHat ; pulled from / inspired by:
https://git.alchemyviewer.org/alchemy/viewer/-/merge_requests/61

In no way do I claim credit for the entirety of this work, however the port for these scripts to sh was my doing. The borrowed work was already on offer to Alchemy for some time as part of the above linked merge request. Inclusion of this work into my refactoring efforts is something I have discussed with both parties.

⚠️ Breaking change for the Linux installer

A large pitfall I see for these changes will be issues with existing installations since previously all Alchemy installs regardless of channel occupied one directory and could not be installed alongside each other.

Users that install with these changes will have an "Alchemy" (old) and an "Alchemy $channel" (new) to contend with. Perhaps something could be done to detect these cases for the default install locations and ask to clean these up, but I am seeking review and input on these changes before moving on to that stage.

How users and devs can test these changes

Please only perform these steps if you both understand them, and are confident in your ability to restore your viewer to a working state should something go wrong. Support will not be provided for these changes at this time, but feedback is welcomed!

Since this does not change any C++ code, just unpack a viewer package, either self-made or one of our official releases, and perform the following modifications before proceeding further:

  1. Replace the contents of alchemy with the contents of wrapper.sh
  2. Replace the contents of the viewer's etc directory with the corresponding scripts in this pull request
  3. Replace the install.sh script with the one from this pull request
  4. Run the install.sh and proceed with installing normally.

You should notice that the install goes into an alchemy-$channel-install directory in your chosen location (/opt or ~/), and that an Alchemy $channel launcher is created for you where you normally launch apps.

@DarlCat DarlCat force-pushed the darl/linux-scripts branch from cfddf2b to c19e055 Compare June 11, 2024 02:09
@FelixWolf
Copy link
Contributor

LGTM

Only concern I have is if the build_data.json format changes, in which it might break this, but if that happens, that should be seen before anything actually gets to the release channel.

Copy link
Member

@VectorMutt VectorMutt left a comment

Choose a reason for hiding this comment

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

LGTM

@VectorMutt VectorMutt merged commit ba00829 into main Jun 11, 2024
@VectorMutt VectorMutt deleted the darl/linux-scripts branch June 11, 2024 03:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants