Support importing factory from https and unrelated fix
Contians fix for #15 (closed) and allows to import factory image from https with checksum verification.
Merge request reports
Activity
Small tip, you can use https://docs.gitlab.com/ee/user/project/issues/managing_issues.html#default-closing-pattern
701 704 import_sn() { 702 705 if [ "x-f" = "x$1" ]; then 703 706 shift 704 TAR="$1" 707 if expr "$1" : "https://" > /dev/null; then -
Do you need to use regexp? How about:
I would even think about supporting in such case other schemas as well
Note that
wgetsupportftpas well. -
No they are supported, not all of them that bash support but they are. Behold: https://pubs.opengroup.org/onlinepubs/9699919799/ and now please use it everywhere.
Want to have some level of security so not supporting http and ftp on purpose.
I still think that you should detect not only
httpsbut URLs in general and report that provided schema is not valid. If you plan on supporting schema then, I think, you should support alsofile://. Ok, would still prefer expr for readability reasons - I'm matching regexp, while your suggestion is to use globs to modify variable and then compare it - somehow less straight forward although faster for computer (as those are builtins), I would argue that slower for average reader. But I can compromise on case.
changed this line in version 3 of the diff
-
I do not think that
expris more readable honestly. It matches regexp without you really using regexp. It is like doinggrepvs.grep -F, both are correct but one of them is better, guess which one. Also my version is technically more correct as it really compares only schema, yours looks for it in whole string (well it would be easy fix but with regexp this is common error).Personally expr usage for me smells like obsolete code. It is probably because in
bashit is fully replaced by[[but when I can I am not using it. Maybe my problem but really here for me it on the other hand decreases readability as I am thinking what are you matching instead that you are looking for substring.
735 753 fi 736 754 } 737 755 756 cleanup() { 757 umount_root 758 remote_unmount 759 [ -z "$TO_DELETE" ] || rm -rf $TO_DELETE changed this line in version 2 of the diff
701 704 import_sn() { 702 705 if [ "x-f" = "x$1" ]; then 703 706 shift 704 TAR="$1" 707 if expr "$1" : "https://" > /dev/null; then 708 local tmpdir="$(mktemp -d)" 709 [ -d "$tmpdir" ] || die "Can't create a tmp directory" 710 TO_DELETE="$TO_DELETE $tmpdir" 711 TAR="$tmpdir/$(basename "$1")" 712 wget -O "$TAR" "$1" || die "Can't donwload '$1'" 713 for sum in md5 sha256; do 714 if wget -O "$TAR"."$sum" "$1"."$sum"; then 715 (cd "$tmpdir"; "$sum"sum -c "$TAR"."$sum" || die "Checksum doesn't match for '$1'") 716 else 717 rm -f "$TAR"."$sum" 718 fi changed this line in version 2 of the diff
assigned to @mhrusecky and unassigned @kkoci
added 1 commit
- d631aa16 - fixup! Import generic config before processing -d
marked as a Work In Progress from d631aa16
-
It is. Checking expected behavior in controlled environment is the test. Yes you should later correlate output of your mocker with real stuff but for preserving expected functionality and checking for introduced errors it is the best option you right now have. Consider unit-tests, those fake pretty much whole application context, those are not tests then?
assigned to @kkoci and unassigned @mhrusecky
assigned to @mhrusecky and unassigned @kkoci
added 1 commit
- ed5e91d1 - fixup! fixup! Import generic config before processing -d
assigned to @kkoci and unassigned @mhrusecky
assigned to @mhrusecky and unassigned @kkoci
mentioned in issue #15 (closed)
