search

gobolinux / Compile

The GoboLinux build tool
52 stars 15 forks source link

Update Compile shell scripts to make use of Bash-5.0 features #47

Closed ermo closed 3 months ago

ermo commented 3 years ago

Motivation

Compile has a long history -- and if we're being honest, it tends to show in the mishmash of styles in the scripts.

Since the plan is to stay with Bash for the most part and only port the more complex scripts to Python3 (per @lucasvr ), it seems like it would make sense to go through all scripts and update them to Bash-5.0 compatible syntax to the benefit of readability and uniformity.

If, during the cleanup process, it turns out that small changes can affect speedups or simplification, that'll just be a nice side benefit.

Style Guide references:

List of Compile scripts

Notes

There doesn't appear to be a rigidly enforced approach to variables -- I've seen all of $var vs. "$var" vs. ${var} vs. "${var}" so far. "$var" and "${var}" are apparently equivalent and I happen to like the latter more as it tends to stand out in code as a variable and works everywhere.

The same can be said about var=`foo` vs. var="`foo`" vs. var=$(foo) vs. var="$(foo)". It seems that var=$(foo) is safe, so that's what I'm going with until something breaks (at which point I'll do "$(foo)" instead).

sage-etcher commented 3 months ago

Are the updated ApplyVariables, AutoPatch, and ColorMake files located in a different branch? the versions in master are still filled with backticks, single bracket comparisons, and the like?

Also, how strictly should the bash style guidelines be adhered to? The 3 updated examples' notes only mention converting `` to $(), logical if/and statements from -o/-a to ||/&&, and logical comparisons from [ $var1 = $var2 ] to [[ "$var1" == "$var2" ]]. However, the guidelines also specify quite a few other changes that could be made, such as (but not limited to):

Just how far are the scripts expected to be updated?

Working Style Guide references: (as of writing this)

Nuc1eoN commented 3 months ago

Are the updated ApplyVariables, AutoPatch, and ColorMake files located in a different branch? the versions in master are still filled with backticks, single bracket comparisons, and the like?

I cannot find them either. I suspect the work will have to be redone :)

Also, how strictly should the bash style guidelines be adhered to? The 3 updated examples' notes only mention converting `` to $(), logical if/and statements from -o/-a to ||/&&, and logical comparisons from [ $var1 = $var2 ] to [[ "$var1" == "$var2" ]]. However, the guidelines also specify quite a few other changes that could be made, such as (but not limited to):

My personal opinion: we should adhere to all of them. That will ensure a level of quality that will ease maintaining and improving Compile in the future -- which is the point of the whole endeavor I guess. Also improve code readability.

Also I would add, the codebase could benefit from descriptive comments and maybe checking it against https://www.shellcheck.net/.

Certainly it means work, but then also, the reward will be immensely greater. Doesn't mean smaller improvements aren't worthwhile, but as @ermo once put it: GoboLinux is a compilation of "good ideas, half-arsed implementations". And for good or bad, I agree. GoboLinux has lots of potential, but the tooling and code quality has really become the limiting factor at this point (in my experience).

The only viable alternative to the above is - in my opinion, implementing Compile in a sane language (like python3, or nim?). But then, that means even more work :man_shrugging:. However, GoboLinux is certainly a distro that has been formed by its community, and therefore you can do whatever feels right for you! If the end results works - and improves the current situation, we take it.

In any case, improving the current codebase is imo a) easier and b) would benefit ANY future work with the code (incl. potentially porting to another language). Therefore I believe at this time, it is be the most pragmatic way to approach it.

A side benefit of this effort might be, getting familiar with the code base of Compile and knowing where to look for potential issues in the future!

Certainly it can sound intimidating. There are easier tasks, like updating the recipe database. However I believe the rewards to improving the codebase will have a big impact on the whole project. Currently GoboLinux is definitely a "DIY-distro", so if you want to see a result you will probably have to put in the work yourself :)

Personally I will work on implementing CI/CD (for recipes and documentation) aswell as updating recipes (a constant task :D). There's other issues I'd love to tackle, but sadly I can't clone myself yet^^

@ermo I know you are knee deep into the Serpent OS project (well done & congrats btw! :clap: ) and I'd love to hear your opinion aswell, if you don't mind.

mwh commented 3 months ago

In all honesty I don't think it'd be worth actually updating everything in this way, as opposed to having new things make use of new baseline functionality where that's available. What is there works, and precisely because shell semantics are so subtle trying to replicate the way that it works will be fraught; it's more likely to introduce errors than remove them. It isn't possible to e.g. make changes from [ to [[ without close understanding of the intended semantics.

The scripts also make heavy use of a large library of functions that are inherently doing things and used in ways that ShellCheck won't like, and that shell style guides deprecate, but they do it deliberately because that's how to produce the correct behaviour. Removing that will be extremely intrusive on perhaps a greater level than rewriting from scratch.

By all means write new code making use of newer functionality, if Bash 5 is going to be the baseline, but I would not encourage mass sweeping updates.

Nuc1eoN commented 3 months ago

@mwh Thank you for pointing that out! :+1:

So I suppose we should focus on bug-fixing the current code, while writing new functionality according to new standards.

Therefore, I am closing this issue.

ermo commented 3 months ago

Are the updated ApplyVariables, AutoPatch, and ColorMake files located in a different branch? the versions in master are still filled with backticks, single bracket comparisons, and the like?

I cannot find them either. I suspect the work will have to be redone :)

Odds are they got stuck on my local ssd install of Gobo, but that disk recently got overwritten.

@ermo I know you are knee deep into the Serpent OS project (well done & congrats btw! 👏 ) and I'd love to hear your opinion aswell, if you don't mind.

Thank you! It's demanding work (and I'm also currently in the middle of driving a long-overdue migration of the Solus py2-based tooling to py3, so Solus can in turn begin preparing its future migration to Serpent tooling).

FWIW, I have no objections to the points @mwh made and am absolutely fine with this issue being closed as a result.

Nuc1eoN commented 3 months ago

Thank you! It's demanding work (and I'm also currently in the middle of driving a long-overdue migration of the Solus py2-based tooling to py3, so Solus can in turn begin preparing its future migration to Serpent tooling).

I can imagine! Can't believe Solus has still been on py2. SerpentOS is a great project, glad to hear you are still involved in pushing the boundaries of the linux ecosystem. :rocket:

FWIW, I have no objections to the points @mwh made and am absolutely fine with this issue being closed as a result.

Thank you :)