Repository: opentofu/opentofu
File: internal/providercache/installer.go (Go)

Code before:
			installTo = i.globalCacheDir
			linkTo = i.targetDir
		} else {
			installTo = i.targetDir
			linkTo = nil // no linking needed
		}

		allowedHashes := preferredHashes
		if mode.forceInstallChecksums() {
			allowedHashes = []getproviders.Hash{}
		}

		authResult, err := installTo.InstallPackage(ctx, meta, allowedHashes)
		if err != nil {
			// TODO: Consider retrying for certain kinds of error that seem
			// likely to be transient. For now, we just treat all errors equally.
			errs[provider] = err
			if cb := evts.FetchPackageFailure; cb != nil {
				cb(provider, version, err)
			}
			continue
		}

		if unlock != nil {
			// Early unlock as the write to the globalCacheDir has completed
			unlock()
			unlock = nil
		}

		new := installTo.ProviderVersion(provider, version)
		if new == nil {
			err := fmt.Errorf("after installing %s it is still not detected in %s; this is a bug in OpenTofu", provider, installTo.BasePath())
			errs[provider] = err
			if cb := evts.FetchPackageFailure; cb != nil {
				cb(provider, version, err)
			}
			continue
		}
		if _, err := new.ExecutableFile(); err != nil {
			err := fmt.Errorf("provider binary not found: %w", err)
			errs[provider] = err
			if cb := evts.FetchPackageFailure; cb != nil {
				cb(provider, version, err)
			}
			continue
		}
		if linkTo != nil {
			// We skip emitting the "LinkFromCache..." events here because
			// it's simpler for the caller to treat them as mutually exclusive.
			// We can just subsume the linking step under the "FetchPackage..."
			// series here (and that's why we use FetchPackageFailure below).

Review comment (refactor):
It seems like there are a few places above where we end the iteration of the loop early without calling `unlock`, which I guess is probably not a huge deal since the process will eventually exit and implicitly release the lock, but it we did a little more work down the path I started in https://github.com/opentofu/opentofu/pull/2166 then we could perhaps factor the body of this loop out into a separate function and use `defer unlock()` to make sure it always gets called on every return path, so that we're not at risk of any quirky behavior in the uncommon cases where we terminate early without unlocking.

(I don't think we should do that refactoring as part of this PR though, since that sort of thing is better reviewed on its own.)

Code after:
	if cb := evts.FetchPackageBegin; cb != nil {
		cb(provider, version, meta.Location)
	}
	var installTo, linkTo *Dir
	if i.globalCacheDir != nil {
		installTo = i.globalCacheDir
		linkTo = i.targetDir
	} else {
		installTo = i.targetDir
		linkTo = nil // no linking needed
	}

	allowedHashes := preferredHashes
	if mode.forceInstallChecksums() {
		allowedHashes = []getproviders.Hash{}
	}

	authResult, err := installTo.InstallPackage(ctx, meta, allowedHashes)
	if err != nil {
		// TODO: Consider retrying for certain kinds of error that seem
		// likely to be transient. For now, we just treat all errors equally.
		if cb := evts.FetchPackageFailure; cb != nil {
			cb(provider, version, err)
		}
		return nil, err
	}

	if unlock != nil {
		// Early unlock as the write to the globalCacheDir has completed
		unlock()
		unlock = nil
	}

	new := installTo.ProviderVersion(provider, version)
	if new == nil {
		err := fmt.Errorf("after installing %s it is still not detected in %s; this is a bug in OpenTofu", provider, installTo.BasePath())
		if cb := evts.FetchPackageFailure; cb != nil {
			cb(provider, version, err)
		}
		return nil, err
	}
	if _, err := new.ExecutableFile(); err != nil {
		err := fmt.Errorf("provider binary not found: %w", err)
		if cb := evts.FetchPackageFailure; cb != nil {
			cb(provider, version, err)
		}
		return nil, err
	}
	if linkTo != nil {
		// We skip emitting the "LinkFromCache..." events here because
		// it's simpler for the caller to treat them as mutually exclusive.