Skip to content
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

Size tests #2240

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Size tests #2240

wants to merge 14 commits into from

Conversation

DennisLfromGA
Copy link
Collaborator

Per Issue #2108 - Add destination size tests:
If the CHROOTS partition size is less than 4 GB, spit out a warning. If CHROOTS free space is less than 2GB, spit out a warning. Also, determine whether the former is due to a Chrubuntu install.

Add routine to check space on CHROOTS partiton and ChrUbuntu partitions
check_space checks that the minimum partition size and free space is met.
check_ubuntu checks if partition 7 is being used and reports it's sizes and configuration
Added return values for check_chrubuntu in case we decide to use them.
'df' command in check_chrubuntu changed to use mountpoint instead of partition.
Moved check_space variable declarations to top of file with the others.
# $1: the partition's minimum size threshold
# $2: the partition's minimum available space threshold
check_space() {
local part=${CHROOTS:-/usr/local}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quoting

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quotes added.

@dnschneid
Copy link
Owner

Thanks for the patch!! Catching up on reviewing things now :)

Updated to incorporate changes per line notes from @dnschneid
Updated to incorporate changes per line notes from @dnschneid
@DennisLfromGA
Copy link
Collaborator Author

Thanks for the patch!! Catching up on reviewing things now :)

Thank you for all the help and time out of your busy schedule. :)

check_space() {
local part="${CHROOTS:-/usr/local}"
df -m $part | awk 'FNR == 2 {print $2,$4}' | {
read -r size avail
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indentation

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

Updated to incorporate changes per line notes from @dnschneid on 12/30/15.
Updated to remove remnant of 'used' on line 280.
# $2: the partition's minimum available space threshold
check_space() {
local part="${CHROOTS:-/usr/local}"
df -m $part | awk 'FNR == 2 {print $2,$4}' | {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quote $part

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quoted.

Updated to incorporate changes per line notes from @dnschneid on 1/1/16.
Removed multiple delays and added message that installation will continue after 5s.
Updated to incorporate changes per line notes from @dnschneid on 1/1/16.
Added missing quotes, fixed indentation, insured lines <=80 chars, sent warnings to stderr.
Also reworked logic to check for existing mounts do to 'ro' mount failing if already mounted.
@DennisLfromGA
Copy link
Collaborator Author

I apologize for all the misstarts ( I'm trying, very trying ;).

@dnschneid
Copy link
Owner

You're doing great!

Simplified logic for invoking check_dualboot function.
un-split warning messages.
removed `proc/mounts` lookup and redid mount logic for mounted and unmounted partition.
'grep' and 'cut' replaced with 'awk'.
@DennisLfromGA
Copy link
Collaborator Author

You're doing great!

Well I certanly don't think so, it's down right embarrassking!

@DennisLfromGA
Copy link
Collaborator Author

I was thinking of changing some of my calculations to use decimals.

The current ones are similar to:

local size_gb="$((${size}/1000))GB"

When size is 19588, it yields '19GB'


I'd like to change them to:

local size_gb="$(echo "$size 1000" | awk '{printf "%.2f\n", $1/$2}') GB"

When size is 19588, it yields '19.58 GB'

I wanted to check with y'all and see if that's something worthwhile doing ???

@tedm
Copy link
Contributor

tedm commented Feb 4, 2016

I would just round to 20GB It's not like you can buy a 19.58GB storage device ?? ;)

@DennisLfromGA
Copy link
Collaborator Author

Okay, thanx. I'll leave them as is then probably.

@dnschneid
Copy link
Owner

If you want it to round up you can do something like:
local size_gb="$(((${size}+500)/1000))GB"

I'm not sure which is preferable...

@zxvv
Copy link
Contributor

zxvv commented Apr 6, 2016

You could also consider using "df -mh" which will give you the size in 'human' readable format, such as 20GB.

@DennisLfromGA
Copy link
Collaborator Author

Yup, thought about that but it's just a little harder to test sizes when in a 'human' readable format.
I'm happy with the way it is now unless we want to add the 'echo_color' attributes before it's merged.

@DennisLfromGA
Copy link
Collaborator Author

I think this is ready to go but you might want to review it again, it's been a while.

@tedm
Copy link
Contributor

tedm commented Oct 25, 2016

@DennisLfromGA Let me know if you want me to test. During some wild attempts to get my Chromebook (Toshiba 2 2015) to run android apps, I lost Crouton, so have to reinstall, probably this weekend.

@DennisLfromGA
Copy link
Collaborator Author

@tedm, Go for it and let us know, thanx.

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

Successfully merging this pull request may close these issues.

4 participants