disable progressbar for mass pdsc downloads, issue #250 (#521)

## Fixes
<!-- List the issue(s) this PR resolves -->
- #250 

## Changes
<!-- List the changes this PR introduces -->
-

## Checklist
<!-- Put an `x` in the boxes. All tasks must be completed and boxes
checked before merging. -->
- [ ] 🤖 This change is covered by unit tests (if applicable).
- [x] 🤹 Manual testing has been performed (if necessary).
- [ ] 🛡️ Security impacts have been considered (if relevant).
- [ ] 📖 Documentation updates are complete (if required).
- [ ] 🧠 Third-party dependencies and TPIP updated (if required).

Author: bgn42

cmd/installer/pack.go

@@ -191,7 +191,7 @@ func (p *PackType) fetch(timeout int) error {
 	log.Debugf("Fetching pack file \"%s\" (or just making sure it exists locally)", p.path)
 	var err error
 	if strings.HasPrefix(p.path, "http") {
-		p.path, err = utils.DownloadFile(p.path, timeout)
+		p.path, err = utils.DownloadFile(p.path, false, timeout)
 		if err == errs.ErrTerminatedByUser {
 			log.Infof("Aborting pack download. Removing \"%s\"", p.path)
 		}

cmd/installer/root.go

@@ -391,7 +391,7 @@ func RemovePdsc(pdscPath string) error {
 //
 //	massDownloadPdscFiles(pdscTag, true, 30)
 func massDownloadPdscFiles(pdscTag xml.PdscTag, skipInstalledPdscFiles bool, timeout int, errTags *lockedSlice) {
-	if err := Installation.downloadPdscFile(pdscTag, skipInstalledPdscFiles, timeout); err != nil {
+	if err := Installation.downloadPdscFile(pdscTag, skipInstalledPdscFiles, true, timeout); err != nil {
 		errTags.lock.Lock()
 		errTags.slice = append(errTags.slice, pdscTag)
 		errTags.lock.Unlock()
@@ -832,7 +832,7 @@ func UpdatePublicIndexIfOnline() error {
 //   - overwrite: A boolean flag to indicate whether to overwrite the existing public index. This will be removed in future versions.
 //   - sparse: A boolean flag to indicate whether to perform a sparse update.
 //   - downloadPdsc: A boolean flag to indicate whether to download PDSC files.
-//   - downloadRemainingPdscFiles: A boolean flag to indicate whether to download remaining PDSC files.
+//   - downloadRemainingPdscFiles: A boolean flag to indicate whether to download all remaining PDSC files.
 //   - concurrency: The number of concurrent operations allowed.
 //   - timeout: The timeout duration for network operations.
 //
@@ -871,7 +871,7 @@ func UpdatePublicIndex(indexPath string, overwrite bool, sparse bool, downloadPd
 			log.Warnf("Non-HTTPS url: \"%s\"", indexPath)
 		}
 
-		indexPath, err = utils.DownloadFile(indexPath, timeout)
+		indexPath, err = utils.DownloadFile(indexPath, false, timeout)
 		if err != nil {
 			return err
 		}
@@ -1306,14 +1306,14 @@ func FindPackURL(pack *PackType) (string, error) {
 			})
 			if len(tags) != 0 {
 				tags[0].Version = ""
-				if err := Installation.downloadPdscFile(tags[0], true, 0); err != nil {
+				if err := Installation.downloadPdscFile(tags[0], true, false, 0); err != nil {
 					return "", err
 				}
 			} else {
 				return "", errs.ErrPdscEntryNotFound
 			}
 		} else {
-			if err := Installation.downloadPdscFile(xml.PdscTag{URL: pack.URL, Vendor: pack.Vendor, Name: pack.Name}, true, 0); err != nil {
+			if err := Installation.downloadPdscFile(xml.PdscTag{URL: pack.URL, Vendor: pack.Vendor, Name: pack.Name}, true, false, 0); err != nil {
 				return "", err
 			}
 		}
@@ -1861,18 +1861,25 @@ func (p *PacksInstallationType) packIsPublic(pack *PackType, pdscTag *xml.PdscTa
 	return true, nil
 }
 
-// downloadPdscFile downloads a PDSC file based on the provided pdscTag and saves it to the specified location.
-// If skipInstalledPdscFiles is true and the file already exists, the function will skip the download.
-// The function also handles switching to a cache URL if necessary and ensures the file is moved to the correct location.
+// downloadPdscFile downloads a PDSC (Pack Description) file from a specified URL and saves it to the local file system.
+// It handles scenarios such as skipping already installed files, switching to a cache URL, and managing file permissions.
 //
 // Parameters:
-//   - pdscTag: An xml.PdscTag containing the vendor, name, and URL of the PDSC file.
-//   - skipInstalledPdscFiles: A boolean indicating whether to skip downloading if the file already exists.
-//   - timeout: An integer specifying the timeout duration for the download.
+//   - pdscTag: An xml.PdscTag object containing metadata about the PDSC file to be downloaded.
+//   - skipInstalledPdscFiles: A boolean flag indicating whether to skip downloading if the PDSC file already exists locally.
+//   - noProgressBar: A boolean flag indicating whether to suppress the progress bar during the download.
+//   - timeout: An integer specifying the timeout duration (in seconds) for the download operation.
 //
 // Returns:
-//   - error: An error if any issues occur during the download or file operations.
-func (p *PacksInstallationType) downloadPdscFile(pdscTag xml.PdscTag, skipInstalledPdscFiles bool, timeout int) error {
+//   - error: An error object if any issues occur during the download or file operations, or nil if the operation succeeds.
+//
+// Behavior:
+//   - If skipInstalledPdscFiles is true and the file already exists locally, the function returns without downloading.
+//   - If the PDSC file's URL is not the default Keil pack root and the public index XML URL matches the default root,
+//     the function switches to the cache URL for downloading.
+//   - The function downloads the PDSC file, temporarily saves it, and then moves it to the target location,
+//     ensuring proper file permissions are set before and after the operation.
+func (p *PacksInstallationType) downloadPdscFile(pdscTag xml.PdscTag, skipInstalledPdscFiles, noProgressBar bool, timeout int) error {
 	basePdscFile := fmt.Sprintf("%s.pdsc", pdscTag.VName())
 	pdscFilePath := filepath.Join(p.WebDir, basePdscFile)
 
@@ -1902,7 +1909,7 @@ func (p *PacksInstallationType) downloadPdscFile(pdscTag xml.PdscTag, skipInstal
 
 	pdscFileURL.Path = path.Join(pdscFileURL.Path, basePdscFile)
 
-	localFileName, err := utils.DownloadFile(pdscFileURL.String(), timeout)
+	localFileName, err := utils.DownloadFile(pdscFileURL.String(), noProgressBar, timeout)
 	defer os.Remove(localFileName)
 
 	if err != nil {
@@ -1964,7 +1971,7 @@ func (p *PacksInstallationType) loadPdscFile(pdscTag xml.PdscTag, timeout int) e
 		return nil
 	}
 
-	localFileName, err := utils.DownloadFile(pdscFileURL.String(), timeout)
+	localFileName, err := utils.DownloadFile(pdscFileURL.String(), false, timeout)
 	defer os.Remove(localFileName)
 
 	if err != nil {

cmd/utils/utils.go

@@ -106,8 +106,25 @@ var (
 	DirModeRW = fs.FileMode(0777)
 )
 
-// DownloadFile downloads a file from an URL and saves it locally under destionationFilePath
-func DownloadFile(URL string, timeout int) (string, error) {
+// DownloadFile downloads a file from the specified URL and saves it to the cache directory.
+// If the file already exists in the cache, it skips the download and returns the cached file path.
+//
+// Parameters:
+//   - URL: The URL of the file to download.
+//   - noProgressBar: A boolean flag to disable the progress bar during the download.
+//   - timeout: The timeout in seconds for the HTTP request. If set to 0, no timeout is applied.
+//
+// Returns:
+//   - string: The file path of the downloaded (or cached) file.
+//   - error: An error if the download or file creation fails.
+//
+// Behavior:
+//   - If the file exists in the cache, it is reused without downloading.
+//   - For HTTPS requests to "https://127.0.0.1", TLS verification is skipped.
+//   - Handles HTTP redirects, cookies, and retries for specific HTTP status codes (e.g., 404, 403).
+//   - Displays a progress bar unless disabled via the noProgressBar parameter or if the terminal is non-interactive.
+//   - Ensures secure file writing and cleans up partially downloaded files in case of errors.
+func DownloadFile(URL string, noProgressBar bool, timeout int) (string, error) {
 	parsedURL, _ := url.Parse(URL)
 	fileBase := path.Base(parsedURL.Path)
 	filePath := filepath.Join(CacheDir, fileBase)
@@ -200,7 +217,7 @@ func DownloadFile(URL string, timeout int) (string, error) {
 			writers = append(writers, progressWriter)
 			instCnt++
 		} else {
-			if IsTerminalInteractive() {
+			if !noProgressBar && IsTerminalInteractive() {
 				progressWriter := progressbar.DefaultBytes(length, "I:")
 				writers = append(writers, progressWriter)
 			}

cmd/utils/utils_test.go

@@ -45,7 +45,7 @@ func TestDownloadFile(t *testing.T) {
 				},
 			),
 		)
-		_, err := utils.DownloadFile(goodServer.URL+"/file.txt", 0)
+		_, err := utils.DownloadFile(goodServer.URL+"/file.txt", false, 0)
 		assert.NotNil(err)
 		assert.True(errs.Is(err, errs.ErrFailedCreatingFile))
 		utils.CacheDir = oldCache
@@ -55,7 +55,7 @@ func TestDownloadFile(t *testing.T) {
 		fileName := "file.txt"
 		defer os.Remove(fileName)
 
-		_, err := utils.DownloadFile(fileName, 0)
+		_, err := utils.DownloadFile(fileName, false, 0)
 		assert.NotNil(err)
 		assert.Equal(errors.Unwrap(err), errs.ErrFailedDownloadingFile)
 	})
@@ -71,7 +71,7 @@ func TestDownloadFile(t *testing.T) {
 			),
 		)
 
-		_, err := utils.DownloadFile(notFoundServer.URL+"/"+fileName, 0)
+		_, err := utils.DownloadFile(notFoundServer.URL+"/"+fileName, false, 0)
 		assert.NotNil(err)
 		assert.Equal(errors.Unwrap(err), errs.ErrBadRequest)
 		assert.False(utils.FileExists(fileName))
@@ -89,7 +89,7 @@ func TestDownloadFile(t *testing.T) {
 			),
 		)
 
-		_, err := utils.DownloadFile(bodyErrorServer.URL+"/"+fileName, 0)
+		_, err := utils.DownloadFile(bodyErrorServer.URL+"/"+fileName, false, 0)
 		assert.NotNil(err)
 		assert.True(errs.Is(err, errs.ErrFailedWrittingToLocalFile))
 	})
@@ -106,7 +106,7 @@ func TestDownloadFile(t *testing.T) {
 			),
 		)
 		url := goodServer.URL + "/" + fileName
-		_, err1 := utils.DownloadFile(url, 0)
+		_, err1 := utils.DownloadFile(url, false, 0)
 		assert.Nil(err1)
 		assert.True(utils.FileExists(fileName))
 		bytes, err2 := os.ReadFile(fileName)
@@ -132,7 +132,7 @@ func TestDownloadFile(t *testing.T) {
 			),
 		)
 		url := goodServer.URL + "/" + fileName
-		_, err1 := utils.DownloadFile(url, 0)
+		_, err1 := utils.DownloadFile(url, false, 0)
 		assert.Nil(err1)
 		assert.True(utils.FileExists(fileName))
 		bytes, err2 := os.ReadFile(fileName)
@@ -161,7 +161,7 @@ func TestDownloadFile(t *testing.T) {
 			),
 		)
 		url := goodServer.URL + "/" + fileName
-		_, err1 := utils.DownloadFile(url, 0)
+		_, err1 := utils.DownloadFile(url, false, 0)
 		assert.Nil(err1)
 		assert.True(utils.FileExists(fileName))
 		bytes, err2 := os.ReadFile(fileName)
@@ -183,7 +183,7 @@ func TestDownloadFile(t *testing.T) {
 			),
 		)
 		url := goodServer.URL + "/" + fileName
-		_, err1 := utils.DownloadFile(url, 0)
+		_, err1 := utils.DownloadFile(url, false, 0)
 		assert.Nil(err1)
 		assert.True(utils.FileExists(fileName))
 		bytes, err2 := os.ReadFile(fileName)
@@ -192,7 +192,7 @@ func TestDownloadFile(t *testing.T) {
 		assert.Equal(1, requestCount)
 
 		// Download it again, this time it shouldn't trigger any HTTP request
-		_, err1 = utils.DownloadFile(url, 0)
+		_, err1 = utils.DownloadFile(url, false, 0)
 		assert.Nil(err1)
 		assert.Equal(1, requestCount)
 	})