feat: Seperate functions for fetching and downloading images #1
Conversation
Signed-off-by: Muhammad <[email protected]>
| @@ -17,7 +17,7 @@ data_dir="${data_home}/ubports-installer-cli" | |||
|
|
|||
| fetch() { | |||
| url=${1} | |||
| cached="${cache_dir}/${url}" | |||
| cached="${cache_dir}/$(basename "${url}")" | |||
There was a problem hiding this comment.
This will break horribly, there is no guarantee the filenames that will be fetched are unique. The cache directory is not supposed to be pretty, having full URLs there doesn't hurt anything.
There was a problem hiding this comment.
This will break horribly, there is no guarantee the filenames that will be fetched are unique. The cache directory is not supposed to be pretty, having full URLs there doesn't hurt anything.
The old one actually breaks worse, for example when it downloads the channels.json from https://system-image.ubports.com, the command manages to separate the link, 'https:' becomes a directory in which '/system-image.ubports.com' exists and then finally comes the channels.json.
And it won't refetch the file anyway, so I don't see how it breaks...?
There was a problem hiding this comment.
That's not breaking, that's the way it's supposed to work and there is nothing wrong with that - like I said, the directory structure is not supposed to be pretty. And what I mean is that you can for example download the channels list for one device, but since you used basename if you try to do it for another it will use the cached one and you will download files for a wrong device.
There was a problem hiding this comment.
ah, I see, yeah I see how it breaks now
ubcli.sh
Outdated
| if [ -z "${latest_images}" ]; then | ||
| printf "$(ct 'red' true)ERROR:$(ct 'red') images for device" | ||
| printf "$(ct 'orange') ${device}$(ct 'red') on channel" | ||
| printf "$(ct 'orange') ${channel}$(ct 'red') not found!" |
There was a problem hiding this comment.
Clear colors after printing (just $(ct))
There was a problem hiding this comment.
Clear colors after printing (just
$(ct))
Resolved.
| file=$(basename "${files}") | ||
| fetch_persistent_files "${URL}${files}" "${install_dir}/${file}" | ||
| echo "update $file $file.asc" >> "${install_dir}/ubuntu_command" | ||
| wget "${URL}${files}" |
There was a problem hiding this comment.
You could have used the same logic that's in fetch to choose between wget and curl
scripts/download.sh
Outdated
| echo "format system | ||
| $( [ -z ${wipe:+true} ] || echo "format data") | ||
| load_keyring image-master.tar.xz image-master.tar.xz.asc | ||
| load_keyring image-signing.tar.xz image-signing.tar.xz.asc | ||
| mount system" > "${install_dir}/ubuntu_command" | ||
| } | ||
|
|
||
| fill_ubuntu_command() { | ||
| index_url=${1} |
There was a problem hiding this comment.
="${1}"
Also resolved.
Signed-off-by: Muhammad [email protected]