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

Weston config should be loaded on start #499

Closed
Wouter92 opened this Issue Sep 2, 2017 · 32 comments

Comments

Projects
None yet
5 participants
@Wouter92
Contributor

Wouter92 commented Sep 2, 2017

Currently the config of weston is loaded post-install. This is done in postmarketos-ui-weston.post-install.

Whenever changing one of the weston config values in the deviceinfo files, you should explicitly reinstall postmarketos-ui-weston (this is not done automatically). However, this does not make much sense because the deviceinfo file is in a device-* package, so you suspect only having to rebuild the device-* package.

Therefore I propose to load the weston config when starting weston, in the startup_weston.sh script.

@ollieparanoid

This comment has been minimized.

Show comment
Hide comment
@ollieparanoid

ollieparanoid Sep 2, 2017

Member

Thank you for making this issue! After a bit of discussion in the chat, we came up with the following idea:

  • do not duplicate weston configuration variables in the deviceinfo anymore
  • have one /etc/xdg/weston.ini provided by the device- package (problematic packages only, which do not work with the default config!)
  • have one /etc/xdg/weston.ini.default provided by postmarketos-ui-weston, which gets loaded in case the other config file does not exist. It should have a comment explaining everything.
Member

ollieparanoid commented Sep 2, 2017

Thank you for making this issue! After a bit of discussion in the chat, we came up with the following idea:

  • do not duplicate weston configuration variables in the deviceinfo anymore
  • have one /etc/xdg/weston.ini provided by the device- package (problematic packages only, which do not work with the default config!)
  • have one /etc/xdg/weston.ini.default provided by postmarketos-ui-weston, which gets loaded in case the other config file does not exist. It should have a comment explaining everything.
@PureTryOut

This comment has been minimized.

Show comment
Hide comment
@PureTryOut

PureTryOut Sep 11, 2017

Contributor

@craftyguy commented:

The reason we originally decided to not ship a weston.ini for each device and the reason I implemented the original code for installing device-specific weston options into weston.ini is because this will overwrite an existing weston.ini on disk if it exists. One example would be if a user installs pmos, uses it for a while (incl. customizing weston.ini), then received a device package update which would then proceed to blow away their changes.

I'm not entirely sure what to do. I agree with OP that we should not have users reinstall postmarketos-ui-* for a change in a device package, but at the same time this approach would indeed override user customized preferences. I could maybe make the device specific files /etc/xdg/weston/weston.ini.device instead, and check if a regular /etc/xdg/weston/weston.ini (provided by the user itself) exists?

So the order would then be the following:

  • check if /etc/xdg/weston/weston.ini exists
  • if not, check if /etc/xdg/weston/weston.ini.device exists
  • if not, check if /etc/xdg/weston/weston.ini.default exists
Contributor

PureTryOut commented Sep 11, 2017

@craftyguy commented:

The reason we originally decided to not ship a weston.ini for each device and the reason I implemented the original code for installing device-specific weston options into weston.ini is because this will overwrite an existing weston.ini on disk if it exists. One example would be if a user installs pmos, uses it for a while (incl. customizing weston.ini), then received a device package update which would then proceed to blow away their changes.

I'm not entirely sure what to do. I agree with OP that we should not have users reinstall postmarketos-ui-* for a change in a device package, but at the same time this approach would indeed override user customized preferences. I could maybe make the device specific files /etc/xdg/weston/weston.ini.device instead, and check if a regular /etc/xdg/weston/weston.ini (provided by the user itself) exists?

So the order would then be the following:

  • check if /etc/xdg/weston/weston.ini exists
  • if not, check if /etc/xdg/weston/weston.ini.device exists
  • if not, check if /etc/xdg/weston/weston.ini.default exists
@ollieparanoid

This comment has been minimized.

Show comment
Hide comment
@ollieparanoid

ollieparanoid Sep 11, 2017

Member

@craftyguy is right, it would be bad if we overwrote a configuration file modified by the user. However, files in /etc/ do not get overridden by apk (and as far as I know there is no other package manager that does this either, pacman makes .pacnew files for example).

Example: postmarketos-base ships /etc/udev/rules.d/50-firmware.rules, let's override it and then install postmarketos-base:

% pmbootstrap zap
% pmbootstrap chroot
/ # mkdir -p /etc/udev/rules.d
/ # echo "hello" > /etc/udev/rules.d/50-firmware.rules
/ # cat /etc/udev/rules.d/50-firmware.rules
hello
/ # apk add postmarketos-base
/ # cat /etc/udev/rules.d/50-firmware.rules
hello

Additionally, apk has the audit action, which you can use to check for files modified by the user:

/ # apk audit /etc/udev
U etc/udev/rules.d/50-firmware.rules
Member

ollieparanoid commented Sep 11, 2017

@craftyguy is right, it would be bad if we overwrote a configuration file modified by the user. However, files in /etc/ do not get overridden by apk (and as far as I know there is no other package manager that does this either, pacman makes .pacnew files for example).

Example: postmarketos-base ships /etc/udev/rules.d/50-firmware.rules, let's override it and then install postmarketos-base:

% pmbootstrap zap
% pmbootstrap chroot
/ # mkdir -p /etc/udev/rules.d
/ # echo "hello" > /etc/udev/rules.d/50-firmware.rules
/ # cat /etc/udev/rules.d/50-firmware.rules
hello
/ # apk add postmarketos-base
/ # cat /etc/udev/rules.d/50-firmware.rules
hello

Additionally, apk has the audit action, which you can use to check for files modified by the user:

/ # apk audit /etc/udev
U etc/udev/rules.d/50-firmware.rules
@craftyguy

This comment has been minimized.

Show comment
Hide comment
@craftyguy

craftyguy Sep 11, 2017

Member

Does apk create "apknew" or some such files similar to pacnew in pacman?

Member

craftyguy commented Sep 11, 2017

Does apk create "apknew" or some such files similar to pacnew in pacman?

@craftyguy

This comment has been minimized.

Show comment
Hide comment
@craftyguy

craftyguy Sep 11, 2017

Member

@PureTryOut & @ollieparanoid

In addition...

What if I don't use weston at all, why is there a weston.ini created on disk now? Is this going to be expected for all DEs, compositors, and other applications that people want to run in pmOS, where each one needing device-specific config will now require device packages to install config files on disk even if the SW needing it is not installed on the device?

The thing I liked about my implementation was it only created the weston.ini file if the user wants weston.

Member

craftyguy commented Sep 11, 2017

@PureTryOut & @ollieparanoid

In addition...

What if I don't use weston at all, why is there a weston.ini created on disk now? Is this going to be expected for all DEs, compositors, and other applications that people want to run in pmOS, where each one needing device-specific config will now require device packages to install config files on disk even if the SW needing it is not installed on the device?

The thing I liked about my implementation was it only created the weston.ini file if the user wants weston.

@craftyguy

This comment has been minimized.

Show comment
Hide comment
@craftyguy

craftyguy Sep 11, 2017

Member

Oh, one other bit:

@ollieparanoid if what you say is true (and I believe you 😄 ), then this PR does not address OP's request entirely since the device packages would not overwrite / apply any changes to weston.ini on reinstallation since the file already exists on disk from the first installation, if a user made changes to it since then. We should expect users to be making changes to weston.ini after it is on disk, since weston has quite a few options, like keyboard layout, that users might want to change separate from device-specific config.

Member

craftyguy commented Sep 11, 2017

Oh, one other bit:

@ollieparanoid if what you say is true (and I believe you 😄 ), then this PR does not address OP's request entirely since the device packages would not overwrite / apply any changes to weston.ini on reinstallation since the file already exists on disk from the first installation, if a user made changes to it since then. We should expect users to be making changes to weston.ini after it is on disk, since weston has quite a few options, like keyboard layout, that users might want to change separate from device-specific config.

@ollieparanoid

This comment has been minimized.

Show comment
Hide comment
@ollieparanoid

ollieparanoid Sep 11, 2017

Member

Does apk create "apknew" or some such files similar to pacnew in pacman?

Not that I am aware of. These files also have their downside, they clutter the filesystem and not everyone is aware of them - apk's audit might be cleaner there.
There are .apk-new files according to @kaniini.

What if I don't use weston at all, why is there a weston.ini created on disk now? Is this going to be expected for all DEs, compositors, and other applications that people want to run in pmOS, where each one needing device-specific config will now require device packages to install config files on disk even if the SW needing it is not installed on the device?

Yep, that would be the case with the new solution. But these config files are small, and we also configure wifi (or at later points maybe hardware that is less essential), even if the user may not use it. I don't like this aspect either, but I prefer it to duplicating everything in the deviceinfo file, and then generating the config.

then this PR does not address OP's request entirely since the device packages would not overwrite / apply any changes to weston.ini on reinstallation since the file already exists on disk from the first installation, if a user made changes to it since then.

True! But at least then it is how it works everywhere on Linux. And usually you would not make changes to that file, then you would get the update. (No need to believe me there, I wrote all the commands down so one can try it out easily ;) )

You raise some good points there, looks like we should discuss this further. My opinion is: I'd rather have the weston.ini everywhere rather than duplicating the weston.ini in the deviceinfo and having complex upgrade logic (vs. being consistent with other configs in /etc/ everywhere).

Member

ollieparanoid commented Sep 11, 2017

Does apk create "apknew" or some such files similar to pacnew in pacman?

Not that I am aware of. These files also have their downside, they clutter the filesystem and not everyone is aware of them - apk's audit might be cleaner there.
There are .apk-new files according to @kaniini.

What if I don't use weston at all, why is there a weston.ini created on disk now? Is this going to be expected for all DEs, compositors, and other applications that people want to run in pmOS, where each one needing device-specific config will now require device packages to install config files on disk even if the SW needing it is not installed on the device?

Yep, that would be the case with the new solution. But these config files are small, and we also configure wifi (or at later points maybe hardware that is less essential), even if the user may not use it. I don't like this aspect either, but I prefer it to duplicating everything in the deviceinfo file, and then generating the config.

then this PR does not address OP's request entirely since the device packages would not overwrite / apply any changes to weston.ini on reinstallation since the file already exists on disk from the first installation, if a user made changes to it since then.

True! But at least then it is how it works everywhere on Linux. And usually you would not make changes to that file, then you would get the update. (No need to believe me there, I wrote all the commands down so one can try it out easily ;) )

You raise some good points there, looks like we should discuss this further. My opinion is: I'd rather have the weston.ini everywhere rather than duplicating the weston.ini in the deviceinfo and having complex upgrade logic (vs. being consistent with other configs in /etc/ everywhere).

@craftyguy

This comment has been minimized.

Show comment
Hide comment
@craftyguy

craftyguy Sep 11, 2017

Member

I see your point.

Heres a new idea: Is there a way to implement conf.d -style configuration for weston? I don't think weston supports this, so something like this would need to be created:

(filenames/paths made up, better names can be chosed, just focus on the flow for now :) )

  1. pick a directory for conf files (e.g. /etc/weston/conf.d/)

  2. device package install a device.ini to this directory, weston installs a base.ini.

  3. we create a hook that these packages can call after installation or upgrade, which then parses these ini files and creates a unified weston.ini that weston can use on startup.

I still don't like the idea of device packages spewing config files on disk that users are not likely to need. Wifi is something most folks would require. Weston is only useful as a tech demo and has little or no use outside of that.

Member

craftyguy commented Sep 11, 2017

I see your point.

Heres a new idea: Is there a way to implement conf.d -style configuration for weston? I don't think weston supports this, so something like this would need to be created:

(filenames/paths made up, better names can be chosed, just focus on the flow for now :) )

  1. pick a directory for conf files (e.g. /etc/weston/conf.d/)

  2. device package install a device.ini to this directory, weston installs a base.ini.

  3. we create a hook that these packages can call after installation or upgrade, which then parses these ini files and creates a unified weston.ini that weston can use on startup.

I still don't like the idea of device packages spewing config files on disk that users are not likely to need. Wifi is something most folks would require. Weston is only useful as a tech demo and has little or no use outside of that.

@ollieparanoid

This comment has been minimized.

Show comment
Hide comment
@ollieparanoid

ollieparanoid Sep 11, 2017

Member

Honestly, that sounds even more complicated to me (as in: additional code necessary for the hook), I would rather keep duplicate stuff in the deviceinfo before we do that. But I do not have the grand solution here either.

Member

ollieparanoid commented Sep 11, 2017

Honestly, that sounds even more complicated to me (as in: additional code necessary for the hook), I would rather keep duplicate stuff in the deviceinfo before we do that. But I do not have the grand solution here either.

@craftyguy

This comment has been minimized.

Show comment
Hide comment
@craftyguy

craftyguy Sep 11, 2017

Member

So it sounds like there are two things here:

  1. My interpretation of OP's request: device packages should be allowed to modify weston.ini to enable new features, but the packages should not overwrite anything the user customized in weston.ini

  2. Device packages shouldn't be installing config files for arbitrary software that users are not likely to have installed

For #1, you're going to need some logic involved with complexity > 0. My last idea is how many other packages (udev, networkmanager, systemd, etc) solve this problem

For #2, just because weston is the common way now to get a display going doesn't mean that anyone will be using it a year from now when Plasma, Hildon, ??? and ???? are up and running in pmOS :)

Member

craftyguy commented Sep 11, 2017

So it sounds like there are two things here:

  1. My interpretation of OP's request: device packages should be allowed to modify weston.ini to enable new features, but the packages should not overwrite anything the user customized in weston.ini

  2. Device packages shouldn't be installing config files for arbitrary software that users are not likely to have installed

For #1, you're going to need some logic involved with complexity > 0. My last idea is how many other packages (udev, networkmanager, systemd, etc) solve this problem

For #2, just because weston is the common way now to get a display going doesn't mean that anyone will be using it a year from now when Plasma, Hildon, ??? and ???? are up and running in pmOS :)

@ollieparanoid

This comment has been minimized.

Show comment
Hide comment
@ollieparanoid

ollieparanoid Sep 11, 2017

Member

Completely agree with 2. - and for 1. I'd also like to have a conf.d-style folder for weston. But the point is, that upstream (weston) doesn't ship it that way, so we would need to roll our own stuff. Maybe I am seeing the complexity as too dramatic though, after all we can implement it in postmarketos-ui-weston and don't need to patch weston for that, which is a really good sign. I'll give this some more thought, thanks for your patience 👍

(Also: more opinions welcome, if you guys find a good solution without me, I'm also fine. What I want is a sane community decision and right now it looks to me that we are all a bit uncertain about this, right?)

Member

ollieparanoid commented Sep 11, 2017

Completely agree with 2. - and for 1. I'd also like to have a conf.d-style folder for weston. But the point is, that upstream (weston) doesn't ship it that way, so we would need to roll our own stuff. Maybe I am seeing the complexity as too dramatic though, after all we can implement it in postmarketos-ui-weston and don't need to patch weston for that, which is a really good sign. I'll give this some more thought, thanks for your patience 👍

(Also: more opinions welcome, if you guys find a good solution without me, I'm also fine. What I want is a sane community decision and right now it looks to me that we are all a bit uncertain about this, right?)

@ollieparanoid

This comment has been minimized.

Show comment
Hide comment
@ollieparanoid

ollieparanoid Sep 13, 2017

Member

I have been playing through how I would implement it in my head.

1. Conf.d

This is what you @craftyguy proposed above, but with recommendations for a few more implementation details.

/etc/xdg/weston.d/
	00-base.ini # from postmarketos-ui-weston
	20-device.ini # from the device-package
	30-some-user-settings.ini # optional, created by user

/etc/xdg/weston.ini:
	contains something like:
	# This file isn't getting used by pmOS
	# Instead, we use /etc/xdg/weston.d/.
	# Read xyz for more information.

When we start weston (as configured in postmarketos-ui-weston), we would merge these config files into one and store it somewhere in /tmp/, then start weston with that config.

In order to do that, we could either write a small C program with the C library iniparser for example (which is already packaged for alpine and ~10kb in size) that merges the files. Or we could patch this into weston and try to get it upstreamed (better solution, but a lot of work).

2. Config tarball

If we do not want to automatically generate the weston.ini from the deviceinfo, and do not wish to directly extract the weston.ini to the file system (because it would get extracted even if weston was not installed), then we would need to store it somewhere else.

We could put those config files into a compressed tarball, e.g.: /usr/share/postmarketos/deviceconfigs.tar.gz

A new trigger in postmarketos-base, which gets executed every time something changes in /usr/bin could do the following when weston, kwin, x11, ... just got installed:

  • check if the tarball provides a suitable config
  • if it does not, do nothing
  • if the config file on disk already exists, print a WARNING and don't extract it
  • if it does not exist on disk, extract it

On uninstallation:

  • check if the config file is still there
  • if it was not modified, delete it
  • if it was modified, print out a note and keep it
  • also delete remaining empty folders leading to the config

Finally on change of the deviceconfigs.tar.gz:

  • update all configs, that have been extracted to disk and were not changed
  • (we could check if deviceconfigs were changed by setting their last modified timestamp to 1970-01-01 on extraction, then comparing that.)

I think the trigger can be written with ~200 lines of POSIX shell code, so it is not that complex.

Member

ollieparanoid commented Sep 13, 2017

I have been playing through how I would implement it in my head.

1. Conf.d

This is what you @craftyguy proposed above, but with recommendations for a few more implementation details.

/etc/xdg/weston.d/
	00-base.ini # from postmarketos-ui-weston
	20-device.ini # from the device-package
	30-some-user-settings.ini # optional, created by user

/etc/xdg/weston.ini:
	contains something like:
	# This file isn't getting used by pmOS
	# Instead, we use /etc/xdg/weston.d/.
	# Read xyz for more information.

When we start weston (as configured in postmarketos-ui-weston), we would merge these config files into one and store it somewhere in /tmp/, then start weston with that config.

In order to do that, we could either write a small C program with the C library iniparser for example (which is already packaged for alpine and ~10kb in size) that merges the files. Or we could patch this into weston and try to get it upstreamed (better solution, but a lot of work).

2. Config tarball

If we do not want to automatically generate the weston.ini from the deviceinfo, and do not wish to directly extract the weston.ini to the file system (because it would get extracted even if weston was not installed), then we would need to store it somewhere else.

We could put those config files into a compressed tarball, e.g.: /usr/share/postmarketos/deviceconfigs.tar.gz

A new trigger in postmarketos-base, which gets executed every time something changes in /usr/bin could do the following when weston, kwin, x11, ... just got installed:

  • check if the tarball provides a suitable config
  • if it does not, do nothing
  • if the config file on disk already exists, print a WARNING and don't extract it
  • if it does not exist on disk, extract it

On uninstallation:

  • check if the config file is still there
  • if it was not modified, delete it
  • if it was modified, print out a note and keep it
  • also delete remaining empty folders leading to the config

Finally on change of the deviceconfigs.tar.gz:

  • update all configs, that have been extracted to disk and were not changed
  • (we could check if deviceconfigs were changed by setting their last modified timestamp to 1970-01-01 on extraction, then comparing that.)

I think the trigger can be written with ~200 lines of POSIX shell code, so it is not that complex.

@ollieparanoid

This comment has been minimized.

Show comment
Hide comment
@ollieparanoid

ollieparanoid Sep 13, 2017

Member

Conclusion

Here are the options we have listed in this thread, and I would like to put it to a vote (so we can either merge @PureTryOut's PR soon, or collectively decide that we want to do it another way).

  1. Generate the weston.ini from the deviceinfo file
  2. Include the weston.ini in the device-* packages
  • config tarball:
    • a) yes
    • b) no
  • config files:
    • c) weston.ini (from device package) and weston.ini.default
    • d) weston.ini, weston.ini.device, weston.ini.default (from here)
    • e) conf.d-style: weston.d folder, merge configs before starting weston

Existing implementations:

  • 1. is what we have right now
  • 2. b) c) is in PR #553

Which solution would you prefer?

Member

ollieparanoid commented Sep 13, 2017

Conclusion

Here are the options we have listed in this thread, and I would like to put it to a vote (so we can either merge @PureTryOut's PR soon, or collectively decide that we want to do it another way).

  1. Generate the weston.ini from the deviceinfo file
  2. Include the weston.ini in the device-* packages
  • config tarball:
    • a) yes
    • b) no
  • config files:
    • c) weston.ini (from device package) and weston.ini.default
    • d) weston.ini, weston.ini.device, weston.ini.default (from here)
    • e) conf.d-style: weston.d folder, merge configs before starting weston

Existing implementations:

  • 1. is what we have right now
  • 2. b) c) is in PR #553

Which solution would you prefer?

@craftyguy

This comment has been minimized.

Show comment
Hide comment
@craftyguy

craftyguy Sep 13, 2017

Member

This probably comes as no surprise, but I vote for either:

Generate the weston.ini from the deviceinfo file

or

e) conf.d-style: weston.d folder, merge configs before starting weston

For the reasons I mentioned previously :)

What we have now doesn't address OP's request, which I think is a good request!

I think 'e' is the best compromise best compromise between meeting OP's request (allowing folks to modify config and device packages to also update config), and not tossing around unneeded config files on disk.
The other options just implement what we have now in a different way but don't address OP's request.

Member

craftyguy commented Sep 13, 2017

This probably comes as no surprise, but I vote for either:

Generate the weston.ini from the deviceinfo file

or

e) conf.d-style: weston.d folder, merge configs before starting weston

For the reasons I mentioned previously :)

What we have now doesn't address OP's request, which I think is a good request!

I think 'e' is the best compromise best compromise between meeting OP's request (allowing folks to modify config and device packages to also update config), and not tossing around unneeded config files on disk.
The other options just implement what we have now in a different way but don't address OP's request.

@ollieparanoid

This comment has been minimized.

Show comment
Hide comment
@ollieparanoid

ollieparanoid Sep 13, 2017

Member

Uh sorry that this was unclear, I meant voting with: 1. or 2. (a || b), (c || d || e)
...so could you add a) or b) to your vote for the 2. part, depending on if you like the tarballs thing or not?

Member

ollieparanoid commented Sep 13, 2017

Uh sorry that this was unclear, I meant voting with: 1. or 2. (a || b), (c || d || e)
...so could you add a) or b) to your vote for the 2. part, depending on if you like the tarballs thing or not?

@craftyguy

This comment has been minimized.

Show comment
Hide comment
@craftyguy

craftyguy Sep 13, 2017

Member

Oh, yea that voting structure was a little unclear :)

  1. b) e)

Or in natural language: device-specific weston.ini should be generated by the postmarketos-ui-weston package based on config in deviceinfo (and only if user selects to install this package/weston), and placed into a separate conf file in /etc/xdg/conf.d from user and base conf files.
Based on my understanding of the other options, this is the only one that actually meets OPs request, since user mods to the file after the fact result in any new device-specific config from being dropped during install.

Member

craftyguy commented Sep 13, 2017

Oh, yea that voting structure was a little unclear :)

  1. b) e)

Or in natural language: device-specific weston.ini should be generated by the postmarketos-ui-weston package based on config in deviceinfo (and only if user selects to install this package/weston), and placed into a separate conf file in /etc/xdg/conf.d from user and base conf files.
Based on my understanding of the other options, this is the only one that actually meets OPs request, since user mods to the file after the fact result in any new device-specific config from being dropped during install.

@PureTryOut

This comment has been minimized.

Show comment
Hide comment
@PureTryOut

PureTryOut Sep 14, 2017

Contributor

I'd say 2. b) e)

Contributor

PureTryOut commented Sep 14, 2017

I'd say 2. b) e)

@ollieparanoid

This comment has been minimized.

Show comment
Hide comment
@ollieparanoid

ollieparanoid Sep 14, 2017

Member

@kaniini wrote another alternative in the chat:

i am more in favor of having device-specific policy packages for each package that needs adjustment, using install_if rules

NOTE: We'll probably need to adjust pmbootstrap to handle install_if correctly.

Member

ollieparanoid commented Sep 14, 2017

@kaniini wrote another alternative in the chat:

i am more in favor of having device-specific policy packages for each package that needs adjustment, using install_if rules

NOTE: We'll probably need to adjust pmbootstrap to handle install_if correctly.

@craftyguy

This comment has been minimized.

Show comment
Hide comment
@craftyguy

craftyguy Sep 14, 2017

Member

@ollieparanoid

after talking to @kaniini on chat, I think the install_if way is the way to go.

We break the "only one package per device" philosophy though, but I'm OK with that if we have a sane way to distribute device-specific configurations, if any are required, without having to account for them all in one monolithic device package that would install everything all the time :)

Member

craftyguy commented Sep 14, 2017

@ollieparanoid

after talking to @kaniini on chat, I think the install_if way is the way to go.

We break the "only one package per device" philosophy though, but I'm OK with that if we have a sane way to distribute device-specific configurations, if any are required, without having to account for them all in one monolithic device package that would install everything all the time :)

@ollieparanoid

This comment has been minimized.

Show comment
Hide comment
@ollieparanoid

ollieparanoid Sep 15, 2017

Member

@PureTryOut, do you see it the same way?

EDIT: reading backlog here is how it would work:

install weston and device-samsung-maguro and it pulls in weston-samsung-maguro through reverse dependency

Member

ollieparanoid commented Sep 15, 2017

@PureTryOut, do you see it the same way?

EDIT: reading backlog here is how it would work:

install weston and device-samsung-maguro and it pulls in weston-samsung-maguro through reverse dependency

@PureTryOut

This comment has been minimized.

Show comment
Hide comment
@PureTryOut

PureTryOut Sep 16, 2017

Contributor

Well, it'd be off our goal of one device specific package, but yeah I guess. If that really makes it only install if Weston is specified to be installed by the user that is.

Contributor

PureTryOut commented Sep 16, 2017

Well, it'd be off our goal of one device specific package, but yeah I guess. If that really makes it only install if Weston is specified to be installed by the user that is.

@ollieparanoid

This comment has been minimized.

Show comment
Hide comment
@ollieparanoid

ollieparanoid Sep 16, 2017

Member

I'm glad that we found a solution we can agree on!

Also while we will have multiple packages, the good news is, that we could still have it all in one aport with subpackages. Something like that:

# ...
subpackages="$pkgname-weston"
# ...
weston() {
    install_if="$pkgname weston"
    install -Dm644 $srcdir/weston.ini $subpkgdir/etc/xdg/weston.ini.device
}

This looks pretty clean if you ask me. Let's make a TODO list:

  • Modify one device package to look like the above
  • Check if it works as expected, adjust pmbootstrap code if necessary
  • Make a new PR for the pmbootstrap changes (if any)
  • Make a new PR, that changes all device-packages to that new format.
  • Open a new issue about how to implement a conf.d-style folder for weston, if that is still desired

@PureTryOut, do you want to give this another shot? I can understand if you're frustrated by this right now, in that case I would do it.

Member

ollieparanoid commented Sep 16, 2017

I'm glad that we found a solution we can agree on!

Also while we will have multiple packages, the good news is, that we could still have it all in one aport with subpackages. Something like that:

# ...
subpackages="$pkgname-weston"
# ...
weston() {
    install_if="$pkgname weston"
    install -Dm644 $srcdir/weston.ini $subpkgdir/etc/xdg/weston.ini.device
}

This looks pretty clean if you ask me. Let's make a TODO list:

  • Modify one device package to look like the above
  • Check if it works as expected, adjust pmbootstrap code if necessary
  • Make a new PR for the pmbootstrap changes (if any)
  • Make a new PR, that changes all device-packages to that new format.
  • Open a new issue about how to implement a conf.d-style folder for weston, if that is still desired

@PureTryOut, do you want to give this another shot? I can understand if you're frustrated by this right now, in that case I would do it.

@craftyguy

This comment has been minimized.

Show comment
Hide comment
@craftyguy

craftyguy Sep 22, 2017

Member

@PureTryOut if you aren't working on this, I'd like to give it a try. I feel like this extra work is pretty much my fault for making a big fuss about the original PR earlier and it seems somewhat unfair to have you implement all of this new stuff if you don't want to.
If you are working on this, just let me know!

Member

craftyguy commented Sep 22, 2017

@PureTryOut if you aren't working on this, I'd like to give it a try. I feel like this extra work is pretty much my fault for making a big fuss about the original PR earlier and it seems somewhat unfair to have you implement all of this new stuff if you don't want to.
If you are working on this, just let me know!

@PureTryOut

This comment has been minimized.

Show comment
Hide comment
@PureTryOut

PureTryOut Sep 22, 2017

Contributor

I'm not working on this right now, I'm on holiday, you can give it a shot. 😉

Contributor

PureTryOut commented Sep 22, 2017

I'm not working on this right now, I'm on holiday, you can give it a shot. 😉

@craftyguy

This comment has been minimized.

Show comment
Hide comment
@craftyguy

craftyguy Sep 29, 2017

Member

This doesn't work at all. It seems that the subpackage never gets pulled in. I modified the device-nokia-rx51 APKBUILD to include:

 subpackages="$pkgname-weston"
...
 weston() {
     install_if="$pkgname weston"
     install -Dm644 $srcdir/weston.ini $subpkgdir/etc/xdg/weston/weston.ini
 }

I also tried setting install_if to all these different permutations/options, but none of them resulted in the device-nokia-rx51-weston package being pulled in during the install:

  • install_if=$pkgname=$pkgver-r$pkgrel weston
  • install_if="weston"
  • install_if="busybox"

(the last two were just a desparate attempt to get it to do something. I am out of ideas since it seems like this is how this APKBUID feature should work based on the Alpine reference guide and all APKBUILDs I could find online using install_if.

Member

craftyguy commented Sep 29, 2017

This doesn't work at all. It seems that the subpackage never gets pulled in. I modified the device-nokia-rx51 APKBUILD to include:

 subpackages="$pkgname-weston"
...
 weston() {
     install_if="$pkgname weston"
     install -Dm644 $srcdir/weston.ini $subpkgdir/etc/xdg/weston/weston.ini
 }

I also tried setting install_if to all these different permutations/options, but none of them resulted in the device-nokia-rx51-weston package being pulled in during the install:

  • install_if=$pkgname=$pkgver-r$pkgrel weston
  • install_if="weston"
  • install_if="busybox"

(the last two were just a desparate attempt to get it to do something. I am out of ideas since it seems like this is how this APKBUID feature should work based on the Alpine reference guide and all APKBUILDs I could find online using install_if.

@MartijnBraam

This comment has been minimized.

Show comment
Hide comment
@MartijnBraam

MartijnBraam Sep 29, 2017

Member

Maybe it doesn't pull it in automatically but prevents it from being installed without those dependencies

Member

MartijnBraam commented Sep 29, 2017

Maybe it doesn't pull it in automatically but prevents it from being installed without those dependencies

@craftyguy

This comment has been minimized.

Show comment
Hide comment
@craftyguy

craftyguy Sep 29, 2017

Member

Maybe it doesn't pull it in automatically but prevents it from being installed without those dependencies

If that's the case, then this isn't really a good route to follow after all. The description of install_if on the Alpine seems to suggest that it should be pulled in if the packages listed in that variable are installed:

install_if can be used when a package needs to be installed when some packages are already installed or are in the dependency tree

Member

craftyguy commented Sep 29, 2017

Maybe it doesn't pull it in automatically but prevents it from being installed without those dependencies

If that's the case, then this isn't really a good route to follow after all. The description of install_if on the Alpine seems to suggest that it should be pulled in if the packages listed in that variable are installed:

install_if can be used when a package needs to be installed when some packages are already installed or are in the dependency tree

@MartijnBraam

This comment has been minimized.

Show comment
Hide comment
@MartijnBraam

MartijnBraam Sep 29, 2017

Member

Hmm yeah, seems like the first example you give should work, maybe the install_if variable isn't set correctly in the subpackage metadata after building?

Edit: I've found examples that do the same thing as your example so it should probably work

Member

MartijnBraam commented Sep 29, 2017

Hmm yeah, seems like the first example you give should work, maybe the install_if variable isn't set correctly in the subpackage metadata after building?

Edit: I've found examples that do the same thing as your example so it should probably work

@craftyguy

This comment has been minimized.

Show comment
Hide comment
@craftyguy

craftyguy Sep 29, 2017

Member

The problem is that it doesn't work. So either there's a bug in abuild or apk, or the specification changed somewhere and the wiki (and online examples in other packages) are wrong 😛

Member

craftyguy commented Sep 29, 2017

The problem is that it doesn't work. So either there's a bug in abuild or apk, or the specification changed somewhere and the wiki (and online examples in other packages) are wrong 😛

@ollieparanoid

This comment has been minimized.

Show comment
Hide comment
@ollieparanoid

ollieparanoid Sep 29, 2017

Member

@kaniini, is there something wrong with this APKBUILD, or is this a bug in apk?

Member

ollieparanoid commented Sep 29, 2017

@kaniini, is there something wrong with this APKBUILD, or is this a bug in apk?

@ollieparanoid

This comment has been minimized.

Show comment
Hide comment
@ollieparanoid

ollieparanoid Oct 4, 2017

Member

install_if works for me, see #696.

@craftyguy, do you want to try using that for the configs again (after #696 got merged)?

Member

ollieparanoid commented Oct 4, 2017

install_if works for me, see #696.

@craftyguy, do you want to try using that for the configs again (after #696 got merged)?

craftyguy added a commit that referenced this issue Oct 10, 2017

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

craftyguy added a commit that referenced this issue Oct 11, 2017

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

craftyguy added a commit that referenced this issue Oct 11, 2017

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

craftyguy added a commit that referenced this issue Oct 11, 2017

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

This comment has been minimized.

Show comment
Hide comment
@craftyguy

craftyguy Oct 11, 2017

Member

Implemented in #739, please test!

Member

craftyguy commented Oct 11, 2017

Implemented in #739, please test!

craftyguy added a commit that referenced this issue Oct 12, 2017

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

ollieparanoid added a commit that referenced this issue Oct 20, 2017

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

PureTryOut added a commit that referenced this issue 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.