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

Joplin #26952

Closed
wants to merge 2 commits into from
Closed

Joplin #26952

wants to merge 2 commits into from

Conversation

fosslinux
Copy link
Contributor

This PR adds the joplin-desktop and joplin-cli packages, containing the GUI and CLI versions of Joplin respectively. They are not distributed in the one package as they can and should be installed separately, they are separate applications.

Closes #19585

@fosslinux
Copy link
Contributor Author

I cannot reproduce this travis failure....

@ericonr
Copy link
Member

ericonr commented Dec 5, 2020

sh: electron-builder: command not found

weird...

@ndowens
Copy link
Contributor

ndowens commented Dec 5, 2020

sh: electron-builder: command not found

weird...

I don't know if this would help, but searching I found https://github.com/electron-userland/electron-builder

@fosslinux
Copy link
Contributor Author

... yes, I found that too... and it means nothing, other than that electron-builder exists, and it builds fine on my local machine

@sgn
Copy link
Member

sgn commented Dec 5, 2020

It may relate to the fact that CI run xbps-src as root. I can reproduce in my docker.

@sgn
Copy link
Member

sgn commented Dec 5, 2020

diff --git a/srcpkgs/joplin-desktop/template b/srcpkgs/joplin-desktop/template
index 669756d2b5..410abb23be 100644
--- a/srcpkgs/joplin-desktop/template
+++ b/srcpkgs/joplin-desktop/template
@@ -22,7 +22,7 @@ do_build() {
 	# Remove unused modules
 	rm -rf packages/{app-mobile,app-cli,generator-joplin,app-clipper}
 
-	npm install
+	npm install --unsafe-perm
 	cd packages/app-desktop
 	npm run dist
 }

@sgn
Copy link
Member

sgn commented Dec 5, 2020

Actually, this is what I think better:

diff --git a/srcpkgs/joplin-desktop/template b/srcpkgs/joplin-desktop/template
index 410abb23be..ff200fcd7c 100644
--- a/srcpkgs/joplin-desktop/template
+++ b/srcpkgs/joplin-desktop/template
@@ -22,7 +22,7 @@ do_build() {
 	# Remove unused modules
 	rm -rf packages/{app-mobile,app-cli,generator-joplin,app-clipper}
 
-	npm install --unsafe-perm
+	npm install ${XBPS_ALLOW_CHROOT_BREAKOUT:+--unsafe-perm}
 	cd packages/app-desktop
 	npm run dist
 }

@fosslinux
Copy link
Contributor Author

I agree, thank you. I will fix

@fosslinux
Copy link
Contributor Author

@sgn could you check if something similar happens for mattermost-desktop element-desktop and friends? It may need a similar fix.

srcpkgs/joplin-cli/template Outdated Show resolved Hide resolved
@sgn
Copy link
Member

sgn commented Dec 14, 2020

This could be used to fix cross, but I don't know why npm doesn't pick wrapped pkg-config

diff --git a/srcpkgs/joplin-cli/template b/srcpkgs/joplin-cli/template
index 2f04a10dd0..abfeddbd98 100644
--- a/srcpkgs/joplin-cli/template
+++ b/srcpkgs/joplin-cli/template
@@ -14,6 +14,12 @@ distfiles="https://github.com/laurent22/joplin/archive/v${version}.tar.gz"
 checksum=55aad4fe50e2da980983a69bc7c0870626064db971550d522e266feb17d38916
 python_version=3
 
+if [ "$CROSS_BUILD" ]; then
+	export PKG_CONFIG_SYSROOT_DIR="$XBPS_CROSS_BASE"
+	export PKG_CONFIG_LIBDIR="$XBPS_CROSS_BASE/usr/lib/pkgconfig"
+	export PKG_CONFIG_PATH="$PKG_CONFIG_LIBDIR:$XBPS_CROSS_BASE/usr/share/pkgconfig"
+fi
+
 do_build() {
 	# Remove unused modules
 	rm -rf packages/{app-mobile,app-desktop,app-clipper,generator-joplin}
diff --git a/srcpkgs/joplin-desktop/template b/srcpkgs/joplin-desktop/template
index ff200fcd7c..6fc7ab0faf 100644
--- a/srcpkgs/joplin-desktop/template
+++ b/srcpkgs/joplin-desktop/template
@@ -13,11 +13,18 @@ license="MIT"
 homepage="https://joplinapp.org"
 distfiles="https://github.com/laurent22/joplin/archive/v${version}.tar.gz"
 checksum=55aad4fe50e2da980983a69bc7c0870626064db971550d522e266feb17d38916
+no_generic_pkgconfig_link=yes
 
 export USE_SYSTEM_7ZA="true"
 export USE_SYSTEM_APP_BUILDER="true"
 export USE_SYSTEM_MKSQUASHFS="true"
 
+if [ "$CROSS_BUILD" ]; then
+	export PKG_CONFIG_SYSROOT_DIR="$XBPS_CROSS_BASE"
+	export PKG_CONFIG_LIBDIR="$XBPS_CROSS_BASE/usr/lib/pkgconfig"
+	export PKG_CONFIG_PATH="$PKG_CONFIG_LIBDIR:$XBPS_CROSS_BASE/usr/share/pkgconfig"
+fi
+
 do_build() {
 	# Remove unused modules
 	rm -rf packages/{app-mobile,app-cli,generator-joplin,app-clipper}

@fosslinux
Copy link
Contributor Author

Sorry, I didn't test crosss... oops. Will push fixes now.

@fosslinux
Copy link
Contributor Author

I wonder, can we make CI not fail for a cross dependency...

@fosslinux
Copy link
Contributor Author

Patch dosen't work, will investigate further. Seems to be using host cflags but cross CC.

@fosslinux
Copy link
Contributor Author

No idea what's happening here, somehow -m64 appears out of nowhere, breaking the build. Going to leave joplin-cli as nocross for now.

@fosslinux
Copy link
Contributor Author

WTF is up with i686 CI?

sgn pushed a commit to sgn/void-packages that referenced this pull request Dec 20, 2020
@fosslinux
Copy link
Contributor Author

Ping

@fosslinux fosslinux force-pushed the joplin-new branch 2 times, most recently from 9e23a49 to 8f1a5f6 Compare January 1, 2021 09:47
@fosslinux
Copy link
Contributor Author

fosslinux commented Jan 1, 2021

Updated to 1.5.14, i686 issue could be reproduced there, oddly, and is now fixed. Cross joplin-cli is still an issue. Tests enabled on joplin-desktop as they work but not on joplin-cli as it requires a hard-to-package tool which I might or might not come back to (jest, FWIW).

@fosslinux
Copy link
Contributor Author

musl runtime is broken

@fosslinux
Copy link
Contributor Author

musl has now been fixed on both joplin-desktop and joplin-cli. It has been runtime tested extensively on x86_64{,-musl} and is ready for merge.

@ericonr
Copy link
Member

ericonr commented Feb 1, 2021

Segfaults when launched, on musl + wayland. Haven't tried on X.

@ericonr
Copy link
Member

ericonr commented Apr 18, 2021

@fosslinux are you interested in merging this even with the intermittent status of electron on musl?

It looks generally ok.

@fosslinux
Copy link
Contributor Author

Yes I am still interested in getting this merged.

Perhaps I should update the package before merge though... come back in 24 hrs :P

@fosslinux fosslinux force-pushed the joplin-new branch 2 times, most recently from 1afc690 to 1103360 Compare July 2, 2021 06:16
@fosslinux
Copy link
Contributor Author

Ping.

  • cross has been fixed
  • updated to latest version

@notramo
Copy link

notramo commented Sep 11, 2021

Why does it segfault on musl Wayland? It's probably not because of Electron as Element Desktop runs fine.

@ericonr
Copy link
Member

ericonr commented Sep 11, 2021

Electron is very fiddly on musl. I had crashes with vscode but not element-desktop, for example.

@github-actions
Copy link

Pull Requests become stale 90 days after last activity and are closed 14 days after that. If this pull request is still relevant bump it or assign it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Package request]: Joplin, or Elephant: an open Evernote alternative(s)
5 participants