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

OS: Add Bedrock Linux #1118

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@paradigm
Copy link

paradigm commented Nov 20, 2018

I'd like to add Bedrock Linux support. I believe there were discussions about doing this in the past, but at the time Bedrock was not in a suitable position to cleanly inter-operate with neofetch. We are now beta testing Bedrock Linux 0.7 Poki which includes changes I explicitly made with the aim of getting it to play more nicely with neofetch. I hope to have Poki out of beta and properly released in around a month, and it'd be great if I could show it off with neofetch then.

Looks like a lot of preliminary work was done in the intervening time period:

which makes me hopeful this will go well.

Some background, in case readers are unfamiliar with Bedrock Linux: A Bedrock system is composed of features from other distros. For example, I'm typing this on a system where my init comes from Void Linux, my Xorg server from Debian Stretch, my window manager from Gentoo Linux, and my web browser comes form Arch.

A couple Bedrock specific concepts that I think neofetch should handle:

  • Sometimes users want Bedrock to integrate different distros together, and sometimes they want the given piece of software to be restricted to its native stratum. Bedrock offers a strat -r feature which can be used to disable cross-distro integration. When strat -r is not used, neofetch should report the system as Bedrock and list all of the packages from all of the strata. When strat -r is used, neofetch should report the system as the given stratum's distribution and only count its packages. For example, while neofetch should report the system as Bedrock, strat -r arch neofetch should report the system as Arch Linux. We can detect this strat -r feature is set by looking for the absence of /bedrock/cross/ directories in the $PATH.
  • A Bedrock system can have multiple instances of the same package manager. For example, a Bedrock system could have components from both Debian and Ubuntu, and thus have two dpkg's. Package counts should reflect this.
    • For package counts that are generated by running a command, we can loop over all of the strata and run each package manager with strat -r. For example, we can run strat -r debian dpkg-query -f '.\n' -W and strat -r ubuntu dpkg-query -f '.\n' -W.
    • For package counts that are generated by counting files, we can list all instances of files on all strata by prefixing /bedrock/strata/*/. For example, we can count both /bedrock/strata/gentoo/var/db/pkg/*/*/ and /bedrock/strata/funtoo/var/db/pkg/*/*/

I don't want Bedrock's unusual attributes to make a mess of the very clean neofetch code base. If there is any disagreement with how I implemented these features I'm happy to change things accordingly.

@paradigm paradigm force-pushed the paradigm:master branch 2 times, most recently from 3841a54 to c90a103 Nov 20, 2018

@paradigm

This comment has been minimized.

Copy link
Author

paradigm commented Nov 20, 2018

Looks like the previous commit, 0168516, broke the test suite by having lines that are too long. Could probably refactor it to have some new variable that contains the result of hw_pagesize / 1024 / 1024.

@paradigm paradigm force-pushed the paradigm:master branch from c90a103 to c1e6f56 Nov 21, 2018

@paradigm

This comment has been minimized.

Copy link
Author

paradigm commented Nov 21, 2018

The code to calculate the width of the ASCII logo included backslashes used for escaping in its calculation, which made it overestimate the width of the logo I'm proposing here. I've added a line to squash doubled up backslashes in the calculation which should resolve the concern for specifically the Bedrock logo. It's probably possible to make a more generalized solution for escaped characters than the hard-coded escaped-backslash one I'm proposing here.

@paradigm paradigm force-pushed the paradigm:master branch 2 times, most recently from 6aecc3a to 9ec9a3e Nov 21, 2018

OS: Add Bedrock Linux
A Bedrock Linux is composed of components from other distributions.  It
may have multiple instances of a given package manager.  For example, if
one has components of both Debian and Ubuntu, there will be two apt
executables.  Package counting should take this into consideration.

Bedrock Linux has a feature to temporarily disable cross-stratum
support.  When this feature is active, programs - such as neofetch -
should act as they would on the native stratum; i.e., neofetch should
detect the stratum's distro and not Bedrock Linux itself.  When this
feature is enabled, /bedrock/cross/* directories are removed from the
$PATH.

@paradigm paradigm force-pushed the paradigm:master branch from 9ec9a3e to 0f46c3c Nov 21, 2018

@paradigm

This comment has been minimized.

Copy link
Author

paradigm commented Nov 21, 2018

Apologies for the push spam. Spent some time iterating on the ASCII logo with the Bedrock community. I think we've settled on something, at least for the time being.

Other successfully merged OS additions had screenshots, and so I figured I should add one in case it's a requirement:

Neofetch Bedrock Linux

@konimex konimex added the Enhancement label Nov 27, 2018

@konimex

This comment has been minimized.

Copy link
Collaborator

konimex commented Nov 27, 2018

I think this will need approval from @dylanaraps since it changed the way get_packages() works.

LGTM, though. And good luck for the Bedrock Linux project! I'll definitely try it one day.

@dylanaraps

This comment has been minimized.

Copy link
Owner

dylanaraps commented Nov 27, 2018

@dylanaraps

This comment has been minimized.

Copy link
Owner

dylanaraps commented Dec 15, 2018

I still don’t have access to a computer but I’ll try and get access to a VM on someone else’s so I can get this merged.

@paradigm

This comment has been minimized.

Copy link
Author

paradigm commented Dec 15, 2018

I certainly understand being busy. Don't feel obligated to rush in a merge, especially at the expense of code quality/style preferences. I'm happy to show off Bedrock with neofetch whenever the opportunity manifests.

@dylanaraps

This comment has been minimized.

Copy link
Owner

dylanaraps commented Dec 15, 2018

@dylanaraps

This comment has been minimized.

Copy link
Owner

dylanaraps commented Jan 7, 2019

I posted a reply here a few days ago but I guess it didn't post.

Lets merge this without the get_packages() changes and work on a proper solution to make Bedrock work correctly.

@paradigm

This comment has been minimized.

Copy link
Author

paradigm commented Jan 7, 2019

The way I'm viewing this:

  • If a user finds neofetch does not support Bedrock at all, there's no surprise, as its a niche distro. Either the user will simply use neofetch without explicit bedrock support, or go and find this PR.
  • If a user finds neofetch appears to support Bedrock, but the package counts are broken, it'll look like a bug. In that case I there's a good chance of undue new bug reports, either to you or to me.

Given this, I'm inclined to wait until neofetch fully works under Bedrock before merging in partial, broken support. However, I could be missing some impetus here, e.g. maybe the lack of Bedrock detection is blocking something else. I don't feel very strongly about it either way, and if you're viewing it differently I'm content to go along with your preferences here.

@dylanaraps

This comment has been minimized.

Copy link
Owner

dylanaraps commented Jan 7, 2019

I won't be making a new release until Bedrock is fully supported. It's just easier for me to split this up (if possible).

@paradigm

This comment has been minimized.

Copy link
Author

paradigm commented Jan 7, 2019

Sounds reasonable to me. I'll remove the packages bit from the PR after work today.

@dylanaraps dylanaraps referenced this pull request Jan 7, 2019

Merged

Bedrock Linux support #1154

@dylanaraps

This comment has been minimized.

Copy link
Owner

dylanaraps commented Jan 7, 2019

I've made some minor changes in #1154. I've also got Bedrock up and running and I now understand the reasons for the modification to get_packages(). I'm considering using this full time now as it would make development across distros seamless.

Anyway. Take a look at #1154 and let me know if there are any further changes or additions that need to be made.

@paradigm

This comment has been minimized.

Copy link
Author

paradigm commented Jan 7, 2019

#1154 looks good to me! If you merge that, feel free to close this PR.

Also, happy to hear that Bedrock may be useful to you. I can certainly see how it would be given your work on neofetch. If you run into issues with it, don't hesitate to reach out.

@dylanaraps

This comment has been minimized.

Copy link
Owner

dylanaraps commented Jan 8, 2019

Merged :)

@dylanaraps dylanaraps closed this Jan 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment