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

Fix linux installation #4

Merged
merged 2 commits into from Jun 27, 2020
Merged

Fix linux installation #4

merged 2 commits into from Jun 27, 2020

Conversation

Blacksuan19
Copy link
Contributor

No description provided.

@dragonwocky
Copy link
Member

looks good... I'll check it out properly after school.
however, there actually wasn't actually "linux" support - that check for linux & windows at the same time was intentional, for the WSL.

i didn't know there was a linux app?

@Blacksuan19
Copy link
Contributor Author

There is a Linux app and it's about the same as the windows version thanks to electron.
Am not sure if it's in other distros repositories but on arch Linux it's available from the AUR
https://aur.archlinux.org/packages/notion-app/

@dragonwocky
Copy link
Member

dragonwocky commented Jun 25, 2020

Ah, thanks, looks like it's an unofficial build. There are a couple around - e.g. https://github.com/puneetsl/lotion, https://github.com/JosephDev/notion-linux-wrapper. I don't really have access to a Linux installation I can test a GUI app on at the moment... would you be able to give these all a go and find out which works best and relevant filepaths, etc.?

Also, what's the benefit of repacking the asar? It runs fine without, and I'm going with the idea that the "enhanced/customised" version should be kept easily identifiable as separate to the packed version Notion provides.

@Blacksuan19
Copy link
Contributor Author

alright will test on these and report shortly

@Blacksuan19
Copy link
Contributor Author

Blacksuan19 commented Jun 26, 2020

lotion installs itself in whatever directory it was cloned in so there is no way to determine its exact location, and the other one is a wrapper ie it just opens the site in a window and doesn't have any local resources.

so neither would work with this, I will try to locate lotion installation folder if that's possible

about repacking asar, it didn't start for me without repacking it, seems like the Linux version looks for app.asar specifically and not the app folder
even tho the Linux version is the same as macOS version(it downloads the dmg and changes the file structure to match Linux)

image

I will add a check to only repack if the platform is Linux, that should leave it unchanged for other platforms

@Blacksuan19
Copy link
Contributor Author

just found this: puneetsl/lotion#31
lotion already has a way to inject CSS and js so I don't think I need to add support for it

@dragonwocky
Copy link
Member

dragonwocky commented Jun 26, 2020

Right. Looks like their injection method is actually based off an old version of the enhancer, but yeah I'll leave it alone for now.

Before I merge, couple other things -

Can you revert L32: this will completely break it on windows, as it checks specifically for the WSL. As it checks there specifically for both Windows and Linux, changing that back won't interfere with Linux compatibility.

If you look at L50-92, it checks for the presence of the app.asar file and app as a folder to handle things if the enhancer (presumably a previous version) is already applied, and if an up-to-date enhanced version is not present, it will "clean" old files and attempt to enhance the default Notion app. As-is, it will assume the modified and re-packaged app files are the original and cause all sorts of weird things. Any solutions?

I suggest:

if linux:
* extract
* check `filepath + '/app/ENHANCER_VERSION.txt'`
-- exists and version matches current enhancer_version = continue with enhancement
-- else: clean (remove folder + extract app.asar.bak. code for this should already be present,
   likely just needs to be refactored slightly)

@Blacksuan19
Copy link
Contributor Author

the scripts current behavior always rewrites the backup, I just found the launch file and if pointed to app directory it launches the app without any issues, maybe the customizer could change app.asar to just app, this way no need to repack and the original app.asar always exists

the line second to last is what needs to be changed to unify the behavior on Linux as well

image

@Blacksuan19
Copy link
Contributor Author

ok I've modified the script to patch the launcher but that means the customizer has to be run as root, not a big deal if you ask me

@Blacksuan19 Blacksuan19 changed the title Fix linux installation & pack app after customizing Fix linux installation Jun 26, 2020
Signed-off-by: blacksuan19 <i@blacksuan19.me>
@Blacksuan19
Copy link
Contributor Author

I think it's ready now

  • original app.asar is backed up and never replaced
  • won't break other platforms
  • updates should work just like on other platforms

@dragonwocky
Copy link
Member

Awesome!

Before I merge README will need to be updated with details on Arch Linux app installation (as it's likely people will miss the Linux package since it's not on the website or get confused and think it applies to them when it doesn't).

Could you add that quick? It's probably just one command to install, but I don't have any experience with the AUR, so...

I've found something for Ubuntu too (https://github.com/jaredallard/notion-app), so giving that a test run now.

@Blacksuan19
Copy link
Contributor Author

I'll update the readme shortly
This repo is what the version in the AUR is based on, it works the same way by using macOS dmg

@dragonwocky dragonwocky merged commit aa5df43 into notion-enhancer:master Jun 27, 2020
@dragonwocky dragonwocky added the enhancement New feature or request label Oct 22, 2020
@dragonwocky dragonwocky added this to closed in features Dec 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants