Skip to content

Conversation

@HunterLarco
Copy link
Contributor

@HunterLarco HunterLarco commented Jan 17, 2026

As per #516, the current implementation fails when -source receives a symlink directory. In this PR we solve this by converting symlinks to realpaths when the config is normalized.

Limitations

This change only converts the value of -source to a resolved path. As a result, passing a directory OR symlink directory to -source will behave as expected. However, we leave the walk logic in deployer.go untouched. This means that if -source contains nested symlinks they will not be supported. In truth I feel this is fine, not only is this the current behavior of s3deploy, but likely out of scope for this change as we'd need to recursively walk symlinks and maintain a relationship between the symlinked paths and the original -source root directory.

Backwards compatability

Should impact no existing flows, just adds -source symlink support.

Example / How I tested this

go build

# Test symlink use case.
ln -s lib my-symlink
./s3deploy -bucket example-bucket -source my-symlink -try
rm -r my-symlink

# Test non-symlink use case.
./s3deploy -bucket example-bucket -source lib -try

@HunterLarco HunterLarco marked this pull request as ready for review January 17, 2026 17:19
lib/config.go Outdated
cfg.SourcePath = filepath.Clean(cfg.SourcePath)

// Resolve symlinks
{
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all fine, except this indented code block -- which I can see make sense in some situations, but not for a 3-4 line block like this.

Copy link
Contributor Author

@HunterLarco HunterLarco Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only reason I did this is to scope "err" to avoid variable shadowing with line 263. I've removed the code block and altered line 263 so that it compiles. Let me know if you'd prefer a different approach

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants