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

Add support for device-specific weston config (#499) #739

Merged
merged 10 commits into from Oct 20, 2017

Conversation

@craftyguy
Copy link
Member

craftyguy commented Oct 10, 2017

Fixes #499. See this issue for more details

@craftyguy

This comment has been minimized.

Copy link
Member Author

craftyguy commented Oct 10, 2017

This currently doesn't work due to #731

@montvid

This comment has been minimized.

Copy link
Contributor

montvid commented Oct 12, 2017

nano /tmp/weston.log

Old Xwayland module loading detected: Please use --xwayland command line option or set xwayland=true in the [core] section in weston.ini

Please change from modules=xwayland.so to xwayland=true It would be nice if it was changed in the default weston.ini too.

@craftyguy

This comment has been minimized.

Copy link
Member Author

craftyguy commented Oct 12, 2017

There is no longer going to be a 'default weston.ini' file, that's the whole point of this PR and the discussion in #499.

Please change from modules=xwayland.so to xwayland=true

Done!

@montvid

This comment has been minimized.

Copy link
Contributor

montvid commented Oct 12, 2017

Well I don't think it is practical to have a weston.ini for every device as not many people will be using weston anyway as they need a more proper gui.

@craftyguy

This comment has been minimized.

Copy link
Member Author

craftyguy commented Oct 12, 2017

Go read the discussion in #499, and add any additional comments you might have in that issue.

Add support for device-specific weston config (#499)
Fixes #499. See this issue for more details

@craftyguy craftyguy force-pushed the fix/499-weston-device-subpkg branch from 6c51cec to a15e978 Oct 12, 2017

@drebrez

This comment has been minimized.

Copy link
Member

drebrez commented Oct 12, 2017

@craftyguy closing this PR will break weston in all the devices except N900?
Or weston may work correctly without the config?

@ollieparanoid
Copy link
Member

ollieparanoid left a comment

@drebrez is right, please adjust all devices, which make use of any weston option in their deviceinfo file (except for deviceinfo_weston_pixman_type, which is not (yet?) in the config). Otherwise they will be broken by this PR.

As @montvid says: Most devices have xwayland as opt-in, but it would make more sense to have that as opt-out for devices where it does not work. If we do it like that, we only need to add the weston config to the fewest devices.

As I see it, we have these TODOs:

postmarketos-ui-weston:

  • add default weston config, which enables xwayland
  • add logic to start_weston.sh to use /etc/xdg/weston.ini if it exists, otherwise /etc/xdg/weston.ini.default

each device:

  • add /etc/xdg/weston.ini if it differs from the default config (e.g. xwayland disabled, keyboard layout)
  • remove weston-options from deviceinfo file (all but deviceinfo_weston_pixman_type)

Thank you for your work on this @craftyguy, it will make the device packages much cleaner!

pkgdesc="Nokia N900"
url="https://github.com/postmarketOS"
arch="noarch"
license="MIT"
depends="linux-postmarketos uboot-tools linux-firmware kbd kbd-bkeymaps ofono"
makedepends="uboot-tools kbd kbd-bkeymaps"
install="$pkgname.post-install"
subpackages=""
subpackages="$pkgname-weston:weston:noarch"

This comment has been minimized.

Copy link
@ollieparanoid

ollieparanoid Oct 12, 2017

Member

The :noarch is unnecessary when the parent package has the same architecture.

@craftyguy

This comment has been minimized.

Copy link
Member Author

craftyguy commented Oct 12, 2017

Oh shit, what did I volunteer for? 😛

I would appreciate some help adding this to device packages.. since there are a lot.

ollieparanoid added some commits Oct 17, 2017

@ollieparanoid

This comment has been minimized.

Copy link
Member

ollieparanoid commented Oct 17, 2017

Here we go! I have no idea why one Travis check is failing though, I didn't even touch firmware-huawei-y530 in this PR, and also the checksum are correct in master.

Reviewing this PR and testing it on real devices would be greatly appreciated (and by anyone, not just the people selected for review). I've quickly tested it in qemu-amd64 and it worked for me.

@ollieparanoid ollieparanoid self-assigned this Oct 17, 2017

@craftyguy

This comment has been minimized.

Copy link
Member Author

craftyguy commented Oct 17, 2017

wow, nice work @ollieparanoid !

@drebrez
Copy link
Member

drebrez left a comment

Awesome refactor/cleanup, only did a review, didn't tested anything yet

# xwayland for example) from the device specific package to /etc/xdg/weston.ini.

[core]
modules=xwayland.so

This comment has been minimized.

Copy link
@drebrez

drebrez Oct 17, 2017

Member
[core]
xwayland=true

is not necessary here?

This comment has been minimized.

Copy link
@ollieparanoid

ollieparanoid Oct 17, 2017

Member

Thanks! modules=xwayland.so is the obsolete version, xwayland=true is the proper one, see #739 (comment) -- Fixed.

@@ -0,0 +1,8 @@
# XWayland seems to be broken for this device.

This comment has been minimized.

Copy link
@drebrez

drebrez Oct 17, 2017

Member

missing changes to the aports/device/device-oneplus-bacon/APKBUILD?

This comment has been minimized.

Copy link
@ollieparanoid

ollieparanoid Oct 17, 2017

Member

Good catch, fixed.

ollieparanoid added some commits Oct 17, 2017

@drebrez
Copy link
Member

drebrez left a comment

code looks good but I'm testing it on samsung-i9070 and weston doesn't start.
Seems it tries to load the drm backend.
Still investigating...

@ollieparanoid

This comment has been minimized.

Copy link
Member

ollieparanoid commented Oct 18, 2017

@drebrez wrote:

/etc/xdg/weston.ini is not the default location that weston looks, if no -c ...weston.ini parameter is specified, it uses /etc/xdg/weston/weston.ini

  • use /etc/xdg/weston/ for the configs
@drebrez

This comment has been minimized.

Copy link
Member

drebrez commented Oct 19, 2017

After a lot of testing and git bisecting, I decided to rebuild all the packages from scratch and finally only by adding backend=fbdev-backend.so to the weston.ini.default everything started working again.
Also the packages with the custom weston.ini with xwayland=false they should contain the backend=fbdev-backend.so

@ollieparanoid

This comment has been minimized.

Copy link
Member

ollieparanoid commented Oct 19, 2017

Wow, that you so much for figuring that out! I'll adjust the PR accordingly.

@drebrez

This comment has been minimized.

Copy link
Member

drebrez commented Oct 19, 2017

@ollieparanoid i'm doing it right now

@ollieparanoid

This comment has been minimized.

Copy link
Member

ollieparanoid commented Oct 19, 2017

Even better, thanks \o/

@ollieparanoid
Copy link
Member

ollieparanoid left a comment

@drebrez you rock!

@ollieparanoid ollieparanoid merged commit e7dfe9b into master Oct 20, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@ollieparanoid ollieparanoid deleted the fix/499-weston-device-subpkg branch Oct 20, 2017

PureTryOut added a commit that referenced this pull request Feb 21, 2018

Fix #499: Don't generate weston.ini from the deviceinfo anymore (#739)
@drebrez deserves much credit for this one for all the testing,
bisecting and for fixing everything. Thank you very much!
---
* devices which need a custom weston.ini ship it with a install_if
  subpackage, so it only gets installed when weston is installed. This
  sounds complicated, but is actually pretty clean in the APKBUILD.
* postmarketos-ui-weston: has a weston.ini.default, which enables
  xwayland and uses fbdev as backend (because that's what most
  devices use!). It defaults to the weston.ini.default if there is no
  weston.ini (as installed by the device package).
* changed spaces to tabs for consistency, general minor refactoring of
  device-APKBUILDs
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.