Skip to content
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

roachprod: fix symlink detection #111136

Merged
merged 1 commit into from
Sep 26, 2023

Conversation

renatolabs
Copy link
Contributor

Previously, roachprod put would complain about symlinks if EvalSymlinks(src) != src. However, that is not always a reliable way to determine if a file is a symbolic link. The documentation for EvalSymlinks states:

If path is relative the result will be relative to the current
directory.

Therefore, the condition mentioned above is not accurate. In particular, if one ran the command roachprod put ./cockroach, the symlink warning would always be displayed: EvalSymlinks would return cockroach (./ prefix removed) and the paths would differ.

This commit updates that logic to check for absolute paths instead. If the absolute paths differ, we get the symlink warning, and the warning message now also includes the path to the file that the symlink resolves to.

Epic: none

Release note: None

Previously, roachprod `put` would complain about symlinks if
`EvalSymlinks(src) != src`. However, that is not always a reliable way
to determine if a file is a symbolic link. The documentation for
`EvalSymlinks` states:

> If path is relative the result will be relative to the current
> directory.

Therefore, the condition mentioned above is not accurate. In
particular, if one ran the command `roachprod put ./cockroach`, the
symlink warning would always be displayed: `EvalSymlinks` would return
`cockroach` (`./` prefix removed) and the paths would differ.

This commit updates that logic to check for absolute paths instead. If
the absolute paths differ, we get the symlink warning, and the warning
message now also includes the path to the file that the symlink
resolves to.

Epic: none

Release note: None
@renatolabs renatolabs requested a review from a team as a code owner September 22, 2023 20:21
@renatolabs renatolabs requested review from rachitgsrivastava and smg260 and removed request for a team September 22, 2023 20:21
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@renatolabs
Copy link
Contributor Author

Minor fix as I kept getting that red colored warning even though I had no symlinks 🙂

@srosenberg srosenberg requested review from srosenberg and removed request for rachitgsrivastava September 26, 2023 02:52
@renatolabs renatolabs added the backport-23.1.x Flags PRs that need to be backported to 23.1 label Sep 26, 2023
@renatolabs
Copy link
Contributor Author

TFTR!

bors r=srosenberg

@craig
Copy link
Contributor

craig bot commented Sep 26, 2023

Build succeeded:

@craig craig bot merged commit f94ca65 into cockroachdb:master Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-23.1.x Flags PRs that need to be backported to 23.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants