You are not logged in.

#1 2016-06-15 19:26:46

zerophase
Member
Registered: 2015-09-03
Posts: 228

Checking if there are anyways I could improve this bash script.

I just wrote a bash script that uses reflector to update the rsync servers in use by powerpill. I also have a systemd service and timer for running it weekly.  I'm just looking for advice on how to improve  it.

#!/bin/bash

declare -a servers
servers=($(reflector -p rsync -f 30 -l 20 -p 20 --sort rate | grep -o 'rsync:\/\/.*'))

declare -i i=0
regex='^[[:blank:]]+\"rsync://.*'
( IFS='\n' 
while read -r line 
do
	if [[ ! $line =~ $regex ]]; then 
		echo "$line" 
	elif ((i < ${#servers[@]})) ; then
		lead=$( expr index "$line" "\""  )
	 	server=
		if ((i < ${#servers[@]} - 1)) ; then
			server="${servers[$i]}\","
		else
			server="${servers[$i]}\""
		fi
		echo "${line:0:$lead}${server}"
		((i++))
	fi
done < updateTest > o  
mv o updateTest )

Offline

#2 2016-06-15 20:54:42

eschwartz
Fellow
Registered: 2014-08-08
Posts: 4,097

Re: Checking if there are anyways I could improve this bash script.

Instead of that while loop, why don't you just use something simple like:

printf "\"%s\",\n" "${servers[@]}"|head -c -2

Managing AUR repos The Right Way -- aurpublish (now a standalone tool)

Offline

#3 2016-06-15 22:51:15

Slithery
Administrator
From: Norfolk, UK
Registered: 2013-12-01
Posts: 5,776

Re: Checking if there are anyways I could improve this bash script.

Running it through ShellCheck throws up a couple of pointers.


No, it didn't "fix" anything. It just shifted the brokeness one space to the right. - jasonwryan
Closing -- for deletion; Banning -- for muppetry. - jasonwryan

aur - dotfiles

Offline

#4 2016-06-16 01:05:59

zerophase
Member
Registered: 2015-09-03
Posts: 228

Re: Checking if there are anyways I could improve this bash script.

Eschwartz wrote:

Instead of that while loop, why don't you just use something simple like:

|head -c -2

This use of head just grabs the file from the end? Starting the write at the last lines?

Offline

#5 2016-06-16 01:38:17

eschwartz
Fellow
Registered: 2014-08-08
Posts: 4,097

Re: Checking if there are anyways I could improve this bash script.

printf "\"%s\",\n" "${servers[@]}"|head -c -2

printf -- print out each element in the "servers" array, in the format:

"element1",
"element2",
"element3",
 

Obviously, the trailing comma + linebreak is unwanted and specifically the comma will break json parsing. Rather than looping and checking each array index for the last element in order to switch between echo commands... just take all-but-the-last-two-bytes by piping it to the `head` command.

I don't know of any way to tell printf to use a different format string for the last argument, so that will have to do.


Managing AUR repos The Right Way -- aurpublish (now a standalone tool)

Offline

#6 2016-06-16 01:50:28

zerophase
Member
Registered: 2015-09-03
Posts: 228

Re: Checking if there are anyways I could improve this bash script.

Eschwartz wrote:
printf "\"%s\",\n" "${servers[@]}"|head -c -2

printf -- print out each element in the "servers" array, in the format:

"element1",
"element2",
"element3",
 

Obviously, the trailing comma + linebreak is unwanted and specifically the comma will break json parsing. Rather than looping and checking each array index for the last element in order to switch between echo commands... just take all-but-the-last-two-bytes by piping it to the `head` command.

I don't know of any way to tell printf to use a different format string for the last argument, so that will have to do.

That makes sense with how the server list ends up printing out.

The only thing I'm at a loss for how to do is overwriting part of a file starting at a certain line without a while loop.

Last edited by zerophase (2016-06-16 01:54:15)

Offline

#7 2016-06-16 02:49:51

eschwartz
Fellow
Registered: 2014-08-08
Posts: 4,097

Re: Checking if there are anyways I could improve this bash script.

printf is not a file, it goes to stdout. As I said, it is piped to `head`.
What file do you think is being overwritten?

...

Ah, I think I see it now.
I was having a hard time seeing what all that for loop did. But I guess the "updateTest" file is the powerpill config?
You can probably use something like sed to replace it -- might improve the readability.


Managing AUR repos The Right Way -- aurpublish (now a standalone tool)

Offline

#8 2016-06-16 02:59:14

zerophase
Member
Registered: 2015-09-03
Posts: 228

Re: Checking if there are anyways I could improve this bash script.

Oh I mean I was redirecting the output to the file I'm writing to with >>

I'm Just wondering if there is anyway to redirect to certain lines, other than sed.

Offline

#9 2016-06-16 07:46:40

zerophase
Member
Registered: 2015-09-03
Posts: 228

Re: Checking if there are anyways I could improve this bash script.

I ended up using perl over sed for it. The sed command needed was just more complicated to write out.

#!/bin/bash
# TODO rename to reflector-rsync.sh
declare -a servers
cd /home/zerophase # remove for final iteration
servers=($(reflector -p rsync -f 30 -l 20 -p 20 --sort rate | grep -o 'rsync:\/\/.*'))

linesToWrite=$(printf "\t\t\t\"%s\",\\\n" "${servers[@]}" | head -c -4 ) 
perl -i -0pE 'use strict; my $repo = "\$repo"; my $arch = "\$arch";'\
's|([[:blank:]]+\"rsync:\/\/[^\]]+)|'"${linesToWrite}"'\n\t\t|gs' testSed # change to powerpill.json 

much nicer to read now.

By the way, is there a suggested place to put bash scripts in use by systemd? Just /bin so they're accessible from everywhere, or just /etc/systemd/system?

Offline

#10 2016-06-16 10:14:52

ukhippo
Member
From: Non-paged pool
Registered: 2014-02-21
Posts: 366

Re: Checking if there are anyways I could improve this bash script.

zerophase wrote:

By the way, is there a suggested place to put bash scripts in use by systemd? Just /bin so they're accessible from everywhere, or just /etc/systemd/system?

I created a /etc/systemd/scripts directory for mine.

Offline

#11 2016-06-16 14:14:24

ewaller
Administrator
From: Pasadena, CA
Registered: 2009-07-13
Posts: 19,744

Re: Checking if there are anyways I could improve this bash script.

My only comment is that you create ./o without looking to see if it exists.  Then, at the end, you move it to ./updateTest.  You might consider checking to see if that interim file does not exist before you obliterate it.
Also, are the parentheses starting in front of IFS='\n' and ending after mv o updateTest not superfluous?  [IDK, I am not a Bash Jockey]


Nothing is too wonderful to be true, if it be consistent with the laws of nature -- Michael Faraday
Sometimes it is the people no one can imagine anything of who do the things no one can imagine. -- Alan Turing
---
How to Ask Questions the Smart Way

Offline

#12 2016-06-16 14:26:20

eschwartz
Fellow
Registered: 2014-08-08
Posts: 4,097

Re: Checking if there are anyways I could improve this bash script.

The parentheses run that in a subshell, which is mostly pointless except it means the IFS change only happens in the subshell.
You could get around that by saving and restoring the IFS, assuming there was an actual concern that you'd need a default IFS within the same script anyway... still, as a style thing it isn't the worst idea ever to get used to always putting things back just in case.

Since the whole thing has been replaced by a perl substitution it doesn't matter anymore.

Last edited by eschwartz (2016-06-16 14:27:20)


Managing AUR repos The Right Way -- aurpublish (now a standalone tool)

Offline

#13 2016-06-16 14:36:09

ewaller
Administrator
From: Pasadena, CA
Registered: 2009-07-13
Posts: 19,744

Re: Checking if there are anyways I could improve this bash script.

Eschwartz; Thanks, make sense.


Nothing is too wonderful to be true, if it be consistent with the laws of nature -- Michael Faraday
Sometimes it is the people no one can imagine anything of who do the things no one can imagine. -- Alan Turing
---
How to Ask Questions the Smart Way

Offline

#14 2016-06-16 20:36:34

zerophase
Member
Registered: 2015-09-03
Posts: 228

Re: Checking if there are anyways I could improve this bash script.

I just scoped the IFS since I didn't know if it could potentially escape my script and cause issues elsewhere. Just assumed avoid any potential headaches.

Offline

#15 2016-06-16 21:20:51

eschwartz
Fellow
Registered: 2014-08-08
Posts: 4,097

Re: Checking if there are anyways I could improve this bash script.

Scripts are run as a child bash shell (same as a subshell except subshells also get to see non-exported variables and a few other things). wink Variables cannot escape from a child process to a parent process.

In this case it was unnecessary. But like I said, it isn't a terrible idea to do that on general principle. Though I would use a SAVEIFS="$IFS" to backup and restore rather than adding an unnecessary subshell. It's also more readable.


Managing AUR repos The Right Way -- aurpublish (now a standalone tool)

Offline

Board footer

Powered by FluxBB