[Solved] Asking for recomendations on the shell script formatting.

[jazzbiker]

Greetings!

I have some script. It do everything it must do. But as for me it looks like a heap of junk. What can be done to make it look a little bit better?

Code: [Select]

TNAME=${1##*/}
BNAME=${2##*/}
TDIR=${1%$TNAME}

depends-on ${TDIR}.do..${TNAME}

JOBS=${JOBS:-2}

LOGS=$(lua -e "for i = 1, $JOBS do print(('$TNAME.%d.log'):format(i)) end")

RDIR=$(pwd)

test -n "$TDIR" && cd $TDIR

export MAP_DIR=$(pwd)

REDO_DIRPREFIX=

for LOG in $LOGS
do
        redo -l $LOG -m $TNAME $BNAME &
done

wait

{ echo 'return {'; cat $LOGS; echo '}'; } > $TNAME.log

cd $RDIR

lua log2map.lua $1.log > $3


Thanks in advance!

[CardealRusso]

There's not too much you can do about it, bash are ugly by nature. Beauty is not the purpose but to just work.
I even ask a member of the TinyCore team here if it wouldn't be interesting to migrate some corescripts to Nim. Nim is a high-level programming language that goes very well with TinyCore; Result will be native binaries, fast, small (some binaries may be smaller than some bash scripts) and easily readable source code.

Code: [Select]

function run_redo(name, base) {
  local logs=( "$name.%d.log" )
  for log in "${logs[@]}"; do
    redo -l "$log" -m "$name" "$base" &
  done
  wait
}

function main() {
  local name=$1
  local base=$2
  local output=$3

  run_redo "$name" "$base"

  echo "return {$(cat "${logs[@]}")}" > "$output"
}

main "$@"/\ this is a bash script btw

[jazzbiker]

Hi CardealRusso,

Thanks for reply! Yes, bash looks like overshooting for such chunk. I even try to make such pieces dash-able. So enclosing some code into the function may improve the outlook, agreed. But Your version misses some ugly details, which will damage the nice picture when added (

Nim is compiled into C, so I don't understand what may be its advantages.

Thanks again and best regards!

[jazzbiker]

After removing some boilerplate with traveling through directories my ugly script looks like

Code: [Select]

TNAME=${1##*/}
TDIR=${1%$TNAME}

depends-on ${TDIR}.do..${TNAME}

JOBS=${JOBS:-2}

LOGS=$(lua -e "for i = 1, $JOBS do print(('$1.%d.log'):format(i)) end")

export MAP_DIR=$(test -n "$TDIR" && cd $TDIR; pwd)

for LOG in $LOGS
do
        redo -l $LOG -m $1 $2 &
done

wait

{ echo 'return {'; cat $LOGS; echo '}'; } > $1.log

lua log2map.lua $1.log > $3
which seems to be slightly less ugly.

[GNUser]

Hi, Andrey. I have hundreds of shell scripts. If they look like junk, then I won't understand them or know how to use them if I haven't used them in a while.

Class A recommendations:
- Use a main function
- Group related steps into functions
- Use function names that include a verb and clearly describe what the function does, so that comments become superfluous
- Put script name, version, purpose, and usage example at top of script
- Maximize simplicity and portability by avoiding bashisms unless you absolutely need bash
- Keep user variables and internal script variables (i.e., ones user should not mess with) separate
- Use clear variable names that include a noun
- After your script is done, check it with checkbashisms and shellcheck

Class B recommendations:
- Put main function at the top
- Use Allman braces style because it is trivial to use correctly (only rules are "each brace on its own line", "brace pairs equally indented")

I'm not sure what your script does, so I can't give the script or its functions good names. Here is a first attempt at cleaning it up. Note that this is rough pseudocode that has several obvious errors and doesn't actually work (plus, its stated purpose and usage example are completely made up)--it's just to illustrate the principles:

Code: [Select]

#!/bin/sh

# dependency-checker v1.0
# Purpose: Find all of an extension's dependencies
# Usage example: $ dependency-checker brave-browser

# user variables:
log=/home/andrey/logs/dependency-checker.logs

main()
{
  run_redo "$name" "$base"
  echo "return {$(cat "${logs[@]}")}" > "$output"
}

run_redo()
{
  local logs=( "$name.%d.log" )
  for log in "${logs[@]}"; do
    redo -l "$log" -m "$name" "$base" &
  done
  wait
}

# internal variables:
TNAME=${1##*/}
BNAME=${2##*/}
TDIR=${1%$TNAME}

main "$@"

I try my hardest to make my shell scripts beautiful. The Pragmatic Programmer is the book that I have found to be the most helpful in this regard.

[jazzbiker]

Hi Bruno,

Thanks for the masterclass! As far a s I will be able I'll learn it. Some questions arose, caused with my bad knowledge of shell:

1)

Code: [Select]

"${logs[@]}"
- what's this? CardealRusso also used this construct in his example.
2) logs are declared as local

Code: [Select]

run_redo()
{
  local logs=( "$name.%d.log" )
...
but are used in main():

Code: [Select]

main()
{
  run_redo "$name" "$base"
  echo "return {$(cat "${logs[@]}")}" > "$output"
}
What does "local" means in shell?

Thank You for the time spent and efforts!

[GNUser]

Hi, Andrey. It seems CardealRusso is a bash guy. Bash is not my cup of tea, but "different strokes for different folks"

1) "${foo}" is the safer (better handling of spaces) equivalent of ${var}
$var is shorthand for ${var}
So far, so good.

Code: [Select]

"${foo[@]}" is a bashism that means "all elements of the array foo". POSIX shells (e.g., dash and busybox ash) do not support arrays.

2) local is not POSIX-defined
In shell, variables are global by default. If you want a variable to be local to a function, some shells allow the local varname (or the even less portable local varname=value) construct.

To avoid breakage and maximize portability, I'd avoid using bashisms and POSIX-undefined keywords in shell scripts.

Refs:
https://tldp.org/LDP/abs/html/arrays.html
https://mywiki.wooledge.org/Bashism (see "Arrays" and "Builtins")

----------

P.S. When I was learning shell, I found that these two excellent, succinct pages gave me the most bang for my buck:
https://mywiki.wooledge.org/BashPitfalls
https://mywiki.wooledge.org/Bashism

If you are extremely interested in shell, there is this exhaustive resource (caveats: time consuming, bash-specific):
https://tldp.org/LDP/abs/html/

[jazzbiker]

I even ask a member of the TinyCore team here if it wouldn't be interesting to migrate some corescripts to Nim. Nim is a high-level programming language that goes very well with TinyCore; Result will be native binaries, fast, small (some binaries may be smaller than some bash scripts) and easily readable source code.

Have You done this for yourself and have practical results?

I expect shells are heavy legacy load from ancient times. I use for my personal purposes Lua as scripting language. I've started it as a kind of exercise. And was astonished that common tasks written with the interpreted language are accomplished faster than the same tasks done by specialized utilities written in compiled language - C. I have no explanaitions, simply got this as the fact.

In my opinion if You are interested in some replacement for shell, it must not be compiled language, it should be dynamically typed interpreted with garbage collector. Otherwise - C can do everything.

[GNUser]

common tasks written with the [Lua] interpreted language are accomplished faster than the same tasks done by specialized utilities written in compiled language - C.

I'm astonished by this. I looked at Lua a few years ago when you clued me in. Although shell (system administration) and awk (text munching) can handle all my household's programming needs, I'll take a look at Lua again.

[CardealRusso]

...it must not be compiled language...

I totally agree with you. To be honest, I hadn't considered Lua.
Usually linux distributions use python, but it seems that it would be inappropriate to integrate the python runtime into tinycore (since python is the same size as tinycore).
I considered Nim for its syntax. I strongly believe that concise and readable code greatly facilitates community development. Currently, CoreScripts bash scripts are neither easy to read nor user friendly (though understandable).

  but yes, you are correct about lua, it is a very good and light option.
And you are also correct in requesting examples. I'll try to formulate an example of what it would look like, for example (it could be another one) tce-update in nim or lua (although I don't have a good command of lua)

[Rich]

Hi jazzbiker

... logs are declared as local ... but are used in main(): ...

The busybox ash shell includes  local  as one of its built-ins. Enter help in a terminal:

Code: [Select]

tc@E310:~$ help
Built-in commands:
------------------
        . : [ [[ alias bg break cd chdir command continue echo eval exec
        exit export false fg getopts hash help history jobs kill let
        local printf pwd read readonly return set shift source test times
        trap true type ulimit umask unalias unset wait
tc@E310:~$
Here is an example of local:

Code: [Select]

#!/bin/sh

Var=5

TestLocal()
{
# This echos the global Var.
echo "2 Var=$Var"

# This overrides the global Var with an empty local Var.
local Var

echo "3 Var=$Var"

# This sets the local Var to 3.
Var=3

echo "4 Var=$Var"
}

echo "1 Var=$Var"
TestLocal
echo "5 Var=$Var"
This is the result:

Code: [Select]

tc@E310:~/split$ ./LocalDemo
1 Var=5
2 Var=5
3 Var=
4 Var=3
5 Var=5
tc@E310:~/split$

[jazzbiker]

Currently, CoreScripts bash scripts are neither easy to read nor user friendly (though understandable).

Core scripts have one huge advantage - they work properly ) I don't want to propose repair what's working right. For TinyCore phenomenal stability is absolutely unique. Some features are accessible only in TinyCore.

And even if put the question - what I would like to rewrite in Core? Frankly speaking - nothing. Extending preferably be done in some new ways or new tools, but the Core is wonderful! Stability is easy to destroy but impossible to repair.

If talk about another languages, for example Lua, some things may be taken into consideration. For example I wrote the Lua version of the initial script. Here it is:

Code: [Select]

#!/usr/bin/env lua

local TDir, TName = arg[1]:match("(.-)([^/]*)$")

------------------------------------------------
-- Auxillary function to run complex commands --
------------------------------------------------

local run = function(cmd)
  assert(os.execute(table.concat(cmd)))
end

run{"depends-on ", TDir, ".do..", TName}

------------------------------
-- Gathering necessary data --
------------------------------

local jobs = tonumber(os.getenv("JOBS")) or 2

local map_dir = assert(io.popen("cd " .. TDir .. "./; pwd")):read()


-----------------------
-- Storing log names --
-----------------------

local log_names = {}

for i = 1, jobs do
  log_names[#log_names + 1] = string.format("%s.%d.log", arg[1], i)
end


------------------------------------------
-- Creating and running complex command --
------------------------------------------

local cmd = {}

for i, name in ipairs(log_names) do
  cmd[#cmd + 1] = string.format("redo -l %s -m %s %s & ", name, arg[1], arg[2])
end

cmd[#cmd + 1] = "wait"

run(cmd)

----------------------------------------------
-- Concatenation of partial logs in one log --
----------------------------------------------

local log_name = arg[1] .. ".log"

local flog = io.open(log_name, "w")

flog:write("return {\n")

for i, name in ipairs(log_names) do
  flog:write(io.open(name):read("a"))
end

flog:write("}\n")

flog:close()


---------------------------------
-- Loading the processing code --
---------------------------------

log2map = assert(loadfile("log2map.lua"))

------------------------------------
-- Emulating MAP_DIR env variable --
------------------------------------

local getenv_orig = os.getenv

os.getenv = function(name)
  if name == "MAP_DIR" then
    return map_dir
  end
  return getenv_orig(name)
end

------------------------
-- Redirecting stdout --
------------------------

assert(io.output(arg[3]))

-----------------------
-- Processing itself --
-----------------------

log2map(log_name)


As You can see it is slightly bigger ) Still for me such approach is more comfortable, because everything is clear and all aspects of code are accessible and changeable. No limitations of what can be done - absolutely everything. Which is wrong for shell. Some things are absolutely impossible to do with shell.

[GNUser]

Core scripts have one huge advantage - they work properly ) I don't want to propose repair what's working right. For TinyCore phenomenal stability is absolutely unique. Some features are accessible only in TinyCore.

Wow, well said! Some TCL scripts are not very pretty but they work perfectly and I wouldn't touch them.

I use my TCL-powered laptop and wireless router daily and my overall impression of TCL is that, by minimizing size and complexity, it is the most stable and easy-to-understand GNU/Linux distribution that exists.

P.S. My *nix journey was Debian -> Arch Linux -> OpenBSD -> Devuan -> TCL. I still manage a Devuan machine in my household (wife's laptop) but I, personally, am 100% TCL. Once you're used to TCL, the complexity and size of other distros is just not acceptable.

[jazzbiker]

The busybox ash shell includes  local  as one of its built-ins. Enter help in a terminal:

Thanks for explanations and examples, Rich! It works as expected. But in GNUsers example it was looking strange and I was not sure whether it works in shell the same way as in another languages. But GNUser wrote that it is not POSIX feature and for better portability it must not be relied upon.

Best regards!

[CardealRusso]

Core scripts have one huge advantage - they work properly

I agree. Unfortunately it is sad to see that the latest commits on such scripts go back 10 or more years.

I'm not very familiar with Lua, but it's a surprisingly easy language. Unfortunately, the beginning is interesting but when the lua script ends it is noticeable that its syntax is no longer modern. The constant use of "ends" and other redundancies makes lua outdated.

In case you're curious, I started making a standalone version of tce-size in lua, as a demo. The LOC seems unnecessarily large, but keep in mind that there is no dependency. The script is also incomplete, but is partially working. You can test from https://www.jdoodle.com/execute-lua-online/ and inserting the package name in the argument.

Code: [Select]

function GetRemoteContent(url)
    local isGzipped = url:sub(-3) == ".gz"
    local wgetCmd = string.format("wget -q -O- %s", url)
    if isGzipped then
        wgetCmd = string.format("%s | gunzip", wgetCmd)
    end

    local handle = io.popen(wgetCmd)
    local content = handle:read("*a")
    handle:close()

    if content and #content > 0 then
        return content
    else
        print(string.format("Failed to download content from '%s'.", url))
        return nil
    end
end

function getFileSize(sizelistContent, filename)
    for line in sizelistContent:gmatch("[^\r\n]+") do
        local file, size = line:match("(%S+)%s+(%d+)")
        if file == filename then
            return tonumber(size)
        end
    end
    return 0
end

function FileExist(filename)
    local path = "/etc/sysconfig/tcedir/optional/" .. filename
    local file = io.open(path, "r")
    if file then
        file:close()
        return true
    else
        return false
    end
end

local app = arg[1]

if not app then
    print("Specify extension in command line:")
    print("Example:   /usr/bin/tce-size firefox.tcz.")
    return
end

if not app:match("%.tcz$") then
    app = app .. ".tcz"
end

local sizelistURL = "http://tinycorelinux.net/14.x/x86_64/tcz/sizelist.gz"
local treeURL = string.format("http://tinycorelinux.net/14.x/x86_64/tcz/%s.tree", app)

local sizelistContent = GetRemoteContent(sizelistURL)
local treeContent = GetRemoteContent(treeURL)

local totalSizeBytes = 0
local totalSizeNonExistentBytes = 0
local fileList = {}

for filename in treeContent:gmatch("%s*(.-)%s*\n") do
    if filename:sub(-4) == ".tcz" then
        local sizeBytes = getFileSize(sizelistContent, filename)
        totalSizeBytes = totalSizeBytes + sizeBytes
        table.insert(fileList, {filename = filename, sizeBytes = sizeBytes})
    end
end

local function formatSizeMB(sizeBytes)
    return string.format("%.2f MB", sizeBytes / (1024 * 1024))
end

table.sort(fileList, function(a, b) return a.sizeBytes > b.sizeBytes end)

print("  Filename                                 Size (bytes),   Size (MB)")
print("------------------------------------------------------------------")

for _, fileData in ipairs(fileList) do
    local filename = fileData.filename
    local sizeBytes = fileData.sizeBytes

    local fileExists = FileExist(filename)

    if not fileExists then
        filename = "+" .. filename
        totalSizeNonExistentBytes = totalSizeNonExistentBytes + sizeBytes
    end

    print(string.format("  %-40s %12d, %15s", filename, sizeBytes, formatSizeMB(sizeBytes)))
end

print("\n  Total size (bytes)                      " .. string.format("%12d, %15s", totalSizeBytes, formatSizeMB(totalSizeBytes)))
print("  + Indicates need to download            " .. string.format("%12d, %15s", totalSizeNonExistentBytes, formatSizeMB(totalSizeNonExistentBytes)))