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

Improved script portability #19

Closed
wants to merge 5 commits into from

Conversation

rachelambda
Copy link
Contributor

This PR improves the script portability in a multitude of ways. The script still retains all functionality and will be identical to the end user.

The most significant of which is porting the script from bash to sh, this means that systems which do not have bash preinstalled (e.g alpinelinux) don't have to install bash to run it.

The script has also been changed to support the busybox wget implementation, yet still use the --show-progress flag if present. The script can now also use curl instead of wget if wget is not installed but curl is.

The script now also supports using other methods of running commands as root, meaning users who prefer doas to sudo won't have to install sudo. Additionally, if neither doas nor sudo is found the script will now use su which should be present on all systems.

The scripts indentation has also been fixed so that it's easier to tell what code belongs to which logical block. Multiple prints after each other have been replaced with single multiline prints. Printing to stderr and exiting has been moved into a function. Variables have been quoted in order to avoid unwanted splitting (which could case errors).

@white-gecko
Copy link
Contributor

I've resolved the merge conflict in https://github.com/white-gecko/flash-it/tree/feature/mergePortable

@white-gecko
Copy link
Contributor

white-gecko commented Mar 8, 2021

The build process appeared to terminate correctly but actually did not prepare the SD card for the boot, so it does not boot.

Flashing rootFS...
Archive:  pinephone-1.1-rootfs.zip
  inflating: pinephone-1.1/sfe-pinephone-1.1-3.4.0.24-devel-20210301/Jolla-3.4.0.24-pinephone-1.1-armv7hl.ks  
  inflating: pinephone-1.1/sfe-pinephone-1.1-3.4.0.24-devel-20210301/Jolla-3.4.0.24-pinephone-1.1-armv7hl.packages  
  inflating: pinephone-1.1/sfe-pinephone-1.1-3.4.0.24-devel-20210301/Jolla-3.4.0.24-pinephone-1.1-armv7hl.urls  
  inflating: pinephone-1.1/sfe-pinephone-1.1-3.4.0.24-devel-20210301/sfe-pinephone-1.1-3.4.0.24-devel-20210301.tar.bz2  
pinephone-1.1/sfe-pinephone-1.1-3.4.0.24-devel-20210301/sfe-pinephone-1.1-3.4.0.24-devel-20210301.tar.bz2
Copying kernel to boot partition...
Boot partition mount: ./boot
cp: der Aufruf von stat für './data/boot/*' ist nicht möglich: Datei oder Verzeichnis nicht gefunden
./boot
Cleaning up!
Unmounting /dev/sda
umount: /dev/sda: not mounted.
Unmounting /dev/sda1
Unmounting /dev/sda2
Flashing /dev/sda OK!
You may now remove the SD card and insert it in your Pine64 device!

Sorry, the error output is in German cp: der Aufruf von stat für './data/boot/*' ist nicht möglich: Datei oder Verzeichnis nicht gefunden. It is just tells, that './data/boot/*' does not exist. Actually ./data/boot/ exists and has content, it has to be some problem with the * in /bin/sh in combination with the implementation of as_root.

@white-gecko
Copy link
Contributor

Once this is done, it will also fix #21 and #22.

@rachelambda
Copy link
Contributor Author

working on a fix for the copying issue

@rachelambda
Copy link
Contributor Author

The issue seems to be that data/boot/ is only readable by root, and that the /* expands as the user running the script, not by sudo. Since the user cannot read this directory it expands to ./data/boot/* instead of the list of files. This can just be solved by allowing all users to read the directory before doing the copy.

as_root chmod a+rx "$MOUNT_DATA/boot"

Just gonna go through the script once, flashing to my SD card before I commit the changes.

@rachelambda
Copy link
Contributor Author

Ran the script! It now copies successfully. I'm not to sure what conflicts should be resolved to be able to merge this, but I guess I'll look into it.

@rachelambda
Copy link
Contributor Author

rachelambda commented Mar 9, 2021

There we go! I merged master and redid the changes from the 4 commits since my fork.

flash-it.sh Outdated
printf '%s\n' "Which image do you want to flash?"

: $(( i = 0 ))
for opt in "PinePhone 1.0 (Development) device" "PinePhone 1.1 (Brave Heart) or 1.2 (Community Editions) devicnePhone 1.1 (Brave Heart) or 1.2 (Community Editions) device" "PineTab device" "PineTab Dev device" "Dont Be Evil devkit"; do
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be

for opt in "PinePhone 1.0 (Development) device" "PinePhone 1.1 (Brave Heart) or 1.2 (Community Editions) device" "PineTab device" "PineTab Dev device" "Dont Be Evil devkit"; do

something came out duplicated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you, solved!

@white-gecko
Copy link
Contributor

Flash an boot successful for me.

Copy link
Contributor

@white-gecko white-gecko left a comment

Choose a reason for hiding this comment

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

Looks got to me, the bugs I have discovered have existed before already ;-) (#24)

@rachelambda rachelambda closed this Aug 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants