Remove broken symlinks when updating deps #2842
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,43 @@ set -o pipefail | |
|
|
||
| source $(dirname $0)/../vendor/github.com/knative/test-infra/scripts/library.sh | ||
|
|
||
| # Remove two type of invalid symlinks | ||
| # 1. Broken symlinks | ||
| # 2. Symlinks pointing outside of the source tree | ||
| # We use recursion to find the canonical path of a symlink | ||
| # because both `realpath` and `readlink -m` are not available on Mac. | ||
| function remove_invalid_symlink() { | ||
|
There was a problem hiding this comment. I think this is overkill. You don't need to traverse the directory tree, only check the target. As we discussed offline, parsing ls should be simple and do the trick. Once this is merged, definitely deserves being moved to test-infra, together with a helper for the major features of update-deps.sh. /hold There was a problem hiding this comment. I still don't quite understand that you meant (sorry for not being a shell expert). The command you reference gives a canonical path, however, for symlink even if we don't follow it all the way, we still need to follow it once by By over overkill, could you hint me an oneliner? Also I agree once this is merged I will go ahead with test-infra and refactor here. There was a problem hiding this comment. # Remove symlinks in /vendor that are broken or lead outside the repo.
function remove_broken_symlinks() {
for link in $(find ./vendor -type l); do
local target="$(ls -l ${link})"
target="${target##* -> }"
# Remove broken symlinks
if [[ ! -e ${target} ]]; then
rm -f ${link}
continue
fi
# Get canonical path to target, remove if outside the repo
target="$(cd ${target%/*} && echo $PWD/${target##*/})"
if [[ ${target} != *github.com/knative/* ]]; then
rm -f ${link}
continue
fi
done
}There was a problem hiding this comment. Thanks a lot @adrcunha for the hint! On the other hand however, after using your changes there are some valid symlinks got deleted, see the following log: $ ./hack/update-deps.sh
unlinking broken: ./vendor/github.com/knative/build/test/panic/kodata/LICENSE
unlinking broken: ./vendor/github.com/knative/build/test/workingdir/kodata/LICENSE
unlinking broken: ./vendor/github.com/knative/build/cmd/controller/kodata/LICENSE
unlinking broken: ./vendor/github.com/knative/build/cmd/logs/kodata/LICENSE
unlinking broken: ./vendor/github.com/knative/build/cmd/nop/kodata/LICENSE
unlinking broken: ./vendor/github.com/knative/build/cmd/git-init/kodata/LICENSE
unlinking broken: ./vendor/github.com/knative/build/cmd/creds-init/kodata/LICENSE
unlinking broken: ./vendor/github.com/knative/build/cmd/webhook/kodata/LICENSE
unlinking broken: ./vendor/github.com/prometheus/procfs/fixtures/selfHowever, e.g. https://github.com/knative/serving/blob/master/vendor/github.com/knative/build/test/panic/kodata/LICENSE is actually a valid symlink. Any reason why? There was a problem hiding this comment. There's a bug in my code, There was a problem hiding this comment. Revised code. The real path resolver was buggy as well (I should have tested it when I wrote it the first time). The output now correctly doesn't include # Remove symlinks in /vendor that are broken or lead outside the repo.
function remove_broken_symlinks() {
for link in $(find ./vendor -type l); do
local target="$(ls -l ${link})"
target="${target##* -> }"
# Remove broken symlinks
if [[ ! -e ${link} ]]; then
echo "[DOESN'T EXIST] rm -f ${link}"
continue
fi
# Get canonical path to target, remove if outside the repo
[[ ${target} == /* ]] || target="./${target}"
target="$(cd `dirname ${link}` && cd ${target%/*} && echo $PWD/${target##*/})"
if [[ ${target} != *github.com/knative/* ]]; then
echo "[OUTSIDE TREE] rm -f ${link}"
continue
fi
done
} |
||
| cd ${REPO_ROOT_DIR} | ||
|
|
||
| local target_file=$1 | ||
| if [[ ! -e ${target_file} ]] ; then | ||
| echo "Removing broken symlink:" $1 | ||
| unlink ${REPO_ROOT_DIR}/$1 | ||
| return | ||
| fi | ||
|
|
||
| cd $(dirname ${target_file}) | ||
| target_file=$(basename ${target_file}) | ||
| # Iterate down a [possible] chain of symlinks | ||
| while [[ -L ${target_file} ]] ; do | ||
| target_file=$(readlink ${target_file}) | ||
| cd $(dirname ${target_file}) | ||
| target_file=$(basename ${target_file}) | ||
| done | ||
|
|
||
| # Compute the canonicalized name by finding the physical path | ||
| # for the directory we're in and appending the target file. | ||
| local phys_dir=`pwd -P` | ||
| target_file=${phys_dir}/${target_file} | ||
|
There was a problem hiding this comment. Seems like this is producing a double slash in the output, e.g.: There was a problem hiding this comment. Oops I commented on the wrong line, but this line is affected too: There was a problem hiding this comment. While I agree that the messages help during development, I think this is too verbose. There was a problem hiding this comment. Em... On linux the path looks right... And as @adrcunha commented I will just delete these msg in the followup commit. |
||
|
|
||
| if [[ ${target_file} != *"github.com/knative/serving"* ]]; then | ||
| echo "Removing symlink outside the source tree: " $1 "->" ${target_file} | ||
| unlink ${REPO_ROOT_DIR}/$1 | ||
| fi | ||
| } | ||
|
|
||
|
|
||
|
|
||
| cd ${REPO_ROOT_DIR} | ||
|
|
||
| # Ensure we have everything we need under vendor/ | ||
|
|
@@ -35,3 +72,8 @@ update_licenses third_party/VENDOR-LICENSE "./cmd/*" | |
| # cherrypick of #66078. Remove this once that reaches a client version | ||
| # we have pulled in. | ||
| git apply ${REPO_ROOT_DIR}/hack/66078.patch | ||
|
|
||
| # Remove all invalid symlinks under ./vendor | ||
| for l in $(find vendor/ -type l); do | ||
| remove_invalid_symlink $l | ||
| done | ||
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.
Nit: this is not a recursive function :)