Model file-paths at agent computer using FilePath instead of nio.Path - #304
Conversation
|
Good catch, the controller-vs-agent filesystem mismatch is a real bug and switching to FilePath is the right fix, thanks for scoping this out as its own small PR instead of bundling it into #169, makes it much easier to review. Heads up on timing: I have #306 open (CI running now) which touches this exact method (toSSHAgentPath) for a different bug, empty-string executable from a blank UI field crashing legacy job configs. It only adds one condition to the null-check, so once it merges this should be a trivial rebase, but you'll likely see a small conflict on that line. Happy to handle the rebase myself if that's easier for you, just say the word. |
|
Since I'm the one who created the conflict by merging #306 first, I went ahead and did the rebase myself rather than leave it on your plate. Pushed it to my own fork, not touching your branch at all: https://github.com/ogulcanaydogan/ssh-agent-plugin/tree/rebase-304-onto-master It was a one-line conflict exactly where I expected (the null-check in toSSHAgentPath), resolved by keeping your FilePath signature with my blank-string check added in. Full suite green (50/50). Totally fine to ignore this and do your own rebase if you'd rather, just wanted to save you the trouble in case it's useful, happy to open a PR against your branch instead if that's easier than cherry-picking. |
When using java.nio.file.Path, the OS and file-system of the controller is used, but the model paths in fact refer to the file-system of and files at the executing agent computer. In case of an OS mismatch between controller and agent, this can lead to parsing errors and incorrect environments. E.g. the Linux implementation of java.nio.file.Path cannot handle Windows style paths and usually throws an Exception in that case. By using (remote) hudson.FilePath instead the OS/file-system of the executing agent computer is considered. Furthermore, use the File separator of file-system at the agent computer, not the controller's. Follow-up on - jenkinsci#296
fac2805 to
5a407a9
Compare
Thanks :)
No problem, was easy to resolve. Just rebased and pushed the update.
There are really a lot of things to consider :O Thanks for fixing that! |
3 participants
When using
java.nio.file.Path, the OS and file-system of the controller is used, but the model paths in fact refer to the file-system of and files at the executing agent computer.In case of an OS mismatch between controller and agent, this can lead to parsing errors and incorrect environments.
E.g. the Linux implementation of
java.nio.file.Pathcannot handle Windows style paths and usually throws an Exception in that case.By using (remote)
hudson.FilePathinstead the OS/file-system of the executing agent computer is considered.Furthermore, use the File separator of file-system at the agent computer, not the controller's.
Follow-up on
Testing done
Submitter checklist