-
Notifications
You must be signed in to change notification settings - Fork 117
Refactor shared lib to use fs.FS
#706
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
rektdeckard
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks for the change!
| // Follow symlinks and include the actual file contents | ||
| if info.Mode()&os.ModeSymlink != 0 { | ||
| realPath, err := filepath.EvalSymlinks(path) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to evaluate symlink %s: %w", path, err) | ||
| } | ||
| info, err = os.Stat(realPath) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to stat %s: %w", realPath, err) | ||
| } | ||
| // Open the real file instead of the symlink | ||
| file, err := os.Open(realPath) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to open file %s: %w", realPath, err) | ||
| } | ||
| defer file.Close() | ||
|
|
||
| header, err := tar.FileInfoHeader(info, "") | ||
| if err != nil { | ||
| return fmt.Errorf("failed to create tar header for file %s: %w", path, err) | ||
| } | ||
| header.Name = relPath | ||
| if err := tarWriter.WriteHeader(header); err != nil { | ||
| return fmt.Errorf("failed to write tar header for file %s: %w", path, err) | ||
| } | ||
|
|
||
| // Copy file contents directly without progress bar | ||
| _, err = io.Copy(tarWriter, file) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to copy file content for %s: %w", path, err) | ||
| } | ||
| return nil | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there was at least one user who needed symlinking to support a monorepo with multiple agent projects and shared code...but I guess until we have a better solution for that use-case it is okay to remove this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah hopefully in those cases, the original files are part of the source so this won't be an issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They had shared code and each agent at the same level, so they would have to make some changes. They were doing this:
/shared
/agent-a
agent.py
pyptoject.toml
/agent-b
agent.py
pyptoject.toml
But still think its okay to break. Let's do a minor version bump when we release this.
Refactor shared lib to use
fs.FSinstead ofworkingDirto support both real and virtual file systems.Unlike previous behavior, all symlink files are excluded.
ref HA-304