Skip to content
This repository has been archived by the owner. It is now read-only.

Add xfce4, including configuration that is suitable for touchscreen. #695

Merged
merged 2 commits into from Oct 7, 2017

Conversation

@pavelmachek
Copy link
Member

@pavelmachek pavelmachek commented Oct 4, 2017

Nokia N900 does not have working hardware acceleration, so xfce4 with enlarged buttons is easiest way to usable user interface at the moment.

# In case of failure, restart after 1s
sleep 1
exit
fi

This comment has been minimized.

@z3ntu

z3ntu Oct 4, 2017
Member

Indentation is wrong here

Copy link
Member

@ollieparanoid ollieparanoid left a comment

Thank you for making this PR! I've added a few comments for what I'd like to discuss before merging.

url="https://github.com/pavelmachek/xfce4-phone"
arch="noarch"
license="GPL3"
depends="xfce4 xorg-server xf86-video-vesa xf86-input-evdev xf86-input-mouse xf86-input-keyboard udev xinput"

This comment has been minimized.

@ollieparanoid

ollieparanoid Oct 4, 2017
Member

  • Does xorg-server not get pulled in by xfce4 (or one of its dependencies?)
  • udev is a dependency of postmarketos-base, so we don't need it here
  • xf86-video-vesa xf86-input-evdev xf86-input-mouse xf86-input-keyboard these will get pulled in automatically by postmarketos-base-x11 (so we have them in one place) once #696 gets merged
  • xinput is for testing, and not required for running the desktop normally, right?

With all this considered, we would have (once #696 gets merged, I recommend we wait until then): depends="xfce4"

pkgver=0.0
pkgrel=0
pkgdesc="Meta package for xfce4"
url="https://github.com/pavelmachek/xfce4-phone"

This comment has been minimized.

@ollieparanoid

ollieparanoid Oct 4, 2017
Member

These are just a bunch of config files, so I suggest we put them inside this package instead of hosting them elsewhere. We could also make a subfolder for them (see postmarketos-base for an example). What do you think about that?


package() {
install -D -m644 "$srcdir"/start_xfce4.sh \
"$pkgdir"/etc/profile.d/start_xfce4.sh || return 1

This comment has been minimized.

@ollieparanoid

ollieparanoid Oct 4, 2017
Member

The || return 1 is obsolete.

package() {
install -D -m644 "$srcdir"/start_xfce4.sh \
"$pkgdir"/etc/profile.d/start_xfce4.sh || return 1
USER="$pkgdir"/home/user2

This comment has been minimized.

@ollieparanoid

ollieparanoid Oct 4, 2017
Member

I don't understand why you write something to /home/user2 at all. Shouldn't these config files go to /etc/skel?
You could also put them in /usr/share/... and put them in the current user's home folder (in case they don't exist yet) inside your xinitrc script, just before starting xfce4. This is done in the postmarketos-ui-hildon xinitrc script.

This comment has been minimized.

@pavelmachek

pavelmachek Oct 5, 2017
Author Member

Sorry about that, I forgot to clean up debugging code.

install -D -m644 "$srcdir"/start_xfce4.sh \
"$pkgdir"/etc/profile.d/start_xfce4.sh || return 1
USER="$pkgdir"/home/user2
ASUSER="-D -m644 -o user"

This comment has been minimized.

@ollieparanoid

ollieparanoid Oct 4, 2017
Member

This variable isn't used anywhere else, delete it?

install -d -m755 "$pkgdir"/etc/skel
cp -a "${srcdir}/xfce4-phone-${pkgver}"/config "${USER}"/.config
cp -a "${srcdir}/xfce4-phone-${pkgver}"/config "${USER}"/config-xfce4-phone
#chown -R user $USER/.config

This comment has been minimized.

@ollieparanoid

ollieparanoid Oct 4, 2017
Member

Please remove commented out code.

@pavelmachek
Copy link
Member Author

@pavelmachek pavelmachek commented Oct 5, 2017

Ok, all but one comment. I'd like to keep separate repository for the files. It allows me to keep APKBUILD simple, may allow other projects to use it easily, and provides a place to put code such as top panel.

@ollieparanoid
Copy link
Member

@ollieparanoid ollieparanoid commented Oct 5, 2017

Thanks for making the changes!

may allow other projects to use it easily

That is a good point. The package is called postmarketos-ui-xfce4 though, so I recommend that we put the xfce4-phone repository to the postmarketOS team on GitHub, would that make sense?

As always, we would set it up so you have all necessary rights (e.g. to write to the master branch).

provides a place to put code such as top panel.

It might make sense to put that in an extra package if this becomes something bigger, and possibly upstreaming it to xfce.

@ollieparanoid
Copy link
Member

@ollieparanoid ollieparanoid commented Oct 5, 2017

As discussed in the chat, the repo is now here (you should have write access once you click the invite links, I think you get them per mail):
https://github.com/postmarketOS/xfce4-phone

It would be nice if you could review and test #696, so we can get both merged ASAP.

@drebrez
Copy link
Member

@drebrez drebrez commented Oct 5, 2017

/usr/bin/startxfce4: Starting X server
/etc/X11/xinit/xserverrc: exec: line 2: /usr/bin/X: not found

EDIT: my fault, there was also Xwayland installed, now it fails because of missing modules.
EDIT2: Installing the packages xf86-input-evdev, xf86-video-fbdev solves and start working.

2017-10-06 01 38 04

@drebrez
Copy link
Member

@drebrez drebrez commented Oct 6, 2017

@pavelmachek I suggest adding the xfce4-battery-plugin package as a dependency, it recognize successfully the battery automatically and shows the percentage 👍

@pavelmachek
Copy link
Member Author

@pavelmachek pavelmachek commented Oct 6, 2017

@pavelmachek
Copy link
Member Author

@pavelmachek pavelmachek commented Oct 6, 2017

It seems to work on top of x11-install-if branch; moved repository as discussed on chat.

@@ -0,0 +1,8 @@

if [ "$(id -u)" = "12345" ] && [ "$(tty)" = "/dev/tty1" ]; then
startxfce4

This comment has been minimized.

@drebrez

drebrez Oct 6, 2017
Member

@pavelmachek can you add a redirect of the output to a log file? like it's done for other UIs (see example)

@pavelmachek
Copy link
Member Author

@pavelmachek pavelmachek commented Oct 6, 2017

@ollieparanoid
Copy link
Member

@ollieparanoid ollieparanoid commented Oct 6, 2017

@drebrez: Awesome photo!

@pavelmachek: At least on Android phones, it is usually very hard to see the console, but reading the log can really be useful, as it contains messages that are not in X11's log (example).

I have redirected the logging to ~/x11.log in postmarketos-ui-hildon now, how about we do the same here? It would be consistent then, and this should only be a workaround until we can properly redirect the logging to /var/log with a display manager anyway.

Other than that and rebasing this PR, I'd say let's ship it! 🚢

This should address review comments, logging now goes to a file.
@pavelmachek pavelmachek force-pushed the pavelmachek:xfce4-4 branch from f0a3f2e to fc14377 Oct 6, 2017
@pavelmachek
Copy link
Member Author

@pavelmachek pavelmachek commented Oct 6, 2017

Okay, here you go. This should address review comments, logging now goes to a file.

Oliver Smith
@ollieparanoid ollieparanoid merged commit ff1b96a into postmarketOS:master Oct 7, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ollieparanoid
Copy link
Member

@ollieparanoid ollieparanoid commented Oct 7, 2017

Merged. Thank you very much!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.