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

Code before:
// This is primarily intended for portable unit testing and not particularly
// useful in "real" callers.
func NewDirWithPlatform(baseDir string, platform getproviders.Platform) *Dir {
	return &Dir{
		baseDir:        baseDir,
		targetPlatform: platform,
	}
}

// BasePath returns the filesystem path of the base directory of this
// cache directory.
func (d *Dir) BasePath() string {
	return filepath.Clean(d.baseDir)
}

func (d *Dir) Lock(ctx context.Context, provider addrs.Provider, version getproviders.Version) (func() error, error) {
	providerPath := getproviders.UnpackedDirectoryPathForPackage(d.baseDir, provider, version, d.targetPlatform)
	// If the lockfile is put within the target directory, it can mess with hashing
	// Instead we add a suffix to the last part of the path (targetplatform) and lock that file instead.
	lockFile := providerPath + ".lock"

	log.Printf("[TRACE] Attempting to acquire global provider lock %s", lockFile)

	// Ensure the provider directory exists
	//nolint: mnd // directory permissions
	if err := os.MkdirAll(providerPath, 0755); err != nil {
		return nil, err
	}

	var f *os.File

	// Try to create the lock file, wait up to 1 second for transient errors to clear.
	for timeout := time.After(time.Second); ; {
		var err error

		// Try to get a handle to the file (or create if it does not exist)
		// Sometimes the creates can conflict and will need to be tried multiple times.
		f, err = os.OpenFile(lockFile, os.O_RDWR|os.O_CREATE, 0644)
		if err == nil {
			// We don't defer f.Close() here as we explicitly want to handle it below
			break
		}

		select {
		case <-timeout:
			return nil, err
		case <-ctx.Done():
			return nil, ctx.Err()
		default:
			// Chill for a bit before trying again
			time.Sleep(50 * time.Millisecond)

Review comment (bug):
Creating the full path here means that if something goes wrong below we'll be left with something that appears to our search code to be a provider package directory but that doesn't contain any files, which could potentially confuse other code that tries to analyze this directory.

The new checksum thing you added would probably address the _main_ problem that would cause because you added an exception for the directory being empty anyway, but since this `lockFile` path is in the parent directory anyway I wonder if we could reduce the risk further by passing `filepath.Dir(providerPath)` instead of just `providerPath` here, and then this directory cannot possibly match either the unpacked or packed directory layouts.

([`getproviders.SearchLocalDirectory`](https://github.com/opentofu/opentofu/blob/da1c39260b09860d51bc592494da5627513c5163/internal/getproviders/filesystem_search.go#L27) is the search code I was referring to when making the above statements. That's used elsewhere to find out what's already present in the cache directory.)


Code after:
// This is primarily intended for portable unit testing and not particularly
// useful in "real" callers.
func NewDirWithPlatform(baseDir string, platform getproviders.Platform) *Dir {
	return &Dir{
		baseDir:        baseDir,
		targetPlatform: platform,
	}
}

// BasePath returns the filesystem path of the base directory of this
// cache directory.
func (d *Dir) BasePath() string {
	return filepath.Clean(d.baseDir)
}

func (d *Dir) Lock(ctx context.Context, provider addrs.Provider, version getproviders.Version) (func() error, error) {
	providerPath := getproviders.UnpackedDirectoryPathForPackage(d.baseDir, provider, version, d.targetPlatform)

	// If the lockfile is put within the target directory, it can mess with hashing
	// Instead we add a suffix to the last part of the path (targetplatform) and lock that file instead.
	dirPath := filepath.Dir(providerPath)
	lockFileName := filepath.Base(providerPath) + ".lock"
	lockFile := filepath.Join(dirPath, lockFileName)

	log.Printf("[TRACE] Attempting to acquire global provider lock %s", lockFile)

	// Ensure the provider directory exists
	//nolint: mnd // directory permissions
	if err := os.MkdirAll(dirPath, 0755); err != nil {
		return nil, err
	}

	var f *os.File

	// Try to create the lock file, wait up to 1 second for transient errors to clear.
	for timeout := time.After(time.Second); ; {
		var err error

		// Try to get a handle to the file (or create if it does not exist)
		// They will all end up with the same file handle on any correctly implemented filesystem.
		// This is one of the many reasons we recommend users look at the flock support of their
		// networked filesystems when using the global provider cache.
		// Windows: even though out flock creates an exclusive lock, we are still able to open a handle to this file and wait below for the actual lock to be provided.
		// Sometimes the creates can conflict and will need to be tried multiple times (incredibly uncommon).
		f, err = os.OpenFile(lockFile, os.O_RDWR|os.O_CREATE, 0644)
		if err == nil {
			// We don't defer f.Close() here as we explicitly want to handle it below
			break
		}

		select {