Merge #485

485: Enforce files in `src` have distinct names after lowercasing and are allowed on all OS's r=ericphanson a=ericphanson

closes #441

Co-authored-by: Eric Hanson <[email protected]>

Author: bors[bot]

Project.toml

@@ -1,7 +1,7 @@
 name = "RegistryCI"
 uuid = "0c95cc5f-2f7e-43fe-82dd-79dbcba86b32"
 authors = ["Dilum Aluthge <[email protected]>", "Fredrik Ekre <[email protected]>", "contributors"]
-version = "7.9.0"
+version = "8.0.0"
 
 [deps]
 Base64 = "2a0f44e3-6c83-55bd-87e4-b1978d98bd5f"

src/AutoMerge/guidelines.jl

@@ -549,6 +549,69 @@ const guideline_code_can_be_downloaded = Guideline(;
     ),
 )
 
+function _find_lowercase_duplicates(v)
+    elts = Dict{String, String}()
+    for x in v
+        lower_x = lowercase(x)
+        if haskey(elts, lower_x)
+            return (elts[lower_x], x)
+        else
+            elts[lower_x] = x
+        end
+    end
+    return nothing
+end
+
+const DISALLOWED_CHARS = ['/', '<', '>', ':', '"', '/', '\\', '|', '?', '*', Char.(0:31)...]
+
+const DISALLOWED_NAMES = ["CON", "PRN", "AUX", "NUL",
+                          ("COM$i" for i in 1:9)...,
+                          ("LPT$i" for i in 1:9)...]
+
+function meets_file_dir_name_check(name)
+    # https://stackoverflow.com/a/31976060
+    idx = findfirst(n -> occursin(n, name), DISALLOWED_CHARS)
+    if idx !== nothing
+        return false, "contains character $(DISALLOWED_CHARS[idx]) which may not be valid as a file or directory name on some platforms"
+    end
+
+    base, ext = splitext(name)
+    if uppercase(name) in DISALLOWED_NAMES || uppercase(base) in DISALLOWED_NAMES
+        return false, "is not allowed"
+    end
+    if endswith(name, ".") || endswith(name, r"\s")
+        return false, "ends with `.` or space"
+    end
+    return true, ""
+end
+
+function meets_src_names_ok(pkg_code_path)
+    src = joinpath(pkg_code_path, "src/")
+    isdir(src) || return false, "`src` directory not found"
+    for (root, dirs, files) in walkdir(src)
+        files_dirs = Iterators.flatten((files, dirs))
+        result = _find_lowercase_duplicates(files_dirs)
+        if result !== nothing
+            x = joinpath(root, result[1])
+            y = joinpath(root, result[2])
+            return false, "Found files or directories in `src` which will cause problems on case insensitive filesystems: `$x` and `$y`"
+        end
+
+        for f in files_dirs
+            ok, msg = meets_file_dir_name_check(f)
+            if !ok
+                return false, "the name of file or directory $(joinpath(root, f)) $(msg). This can cause problems on some operating systems or file systems."
+            end
+        end
+    end
+    return true, ""
+end
+
+const guideline_src_names_OK = Guideline(;
+    info="`src` files and directories names are OK",
+    check=data -> meets_src_names_ok(data.pkg_code_path),
+)
+
 function meets_code_can_be_downloaded(registry_head, pkg, version, pr; pkg_code_path)
     uuid, package_repo, subdir, tree_hash_from_toml = parse_registry_pkg_info(
         registry_head, pkg, version
@@ -909,6 +972,7 @@ function get_automerge_guidelines(
         # after `guideline_code_can_be_downloaded` so
         # that it can use the downloaded code!
         (guideline_version_has_osi_license, check_license),
+        (guideline_src_names_OK, true),
         (guideline_version_can_be_imported, true),
         (:update_status, true),
         (guideline_dependency_confusion, true),
@@ -948,6 +1012,7 @@ function get_automerge_guidelines(
         # after `guideline_code_can_be_downloaded` so
         # that it can use the downloaded code!
         (guideline_version_has_osi_license, check_license),
+        (guideline_src_names_OK, true),
         (guideline_version_can_be_imported, true),
     ]
     return guidelines

test/automerge-unit.jl

@@ -246,6 +246,67 @@ end
         @test nothing == @test_nowarn AutoMerge.always_assert(1 == 1)
         @test_throws AutoMerge.AlwaysAssertionError AutoMerge.always_assert(1 == 2)
     end
+    @testset "_find_lowercase_duplicates" begin
+        @test AutoMerge._find_lowercase_duplicates(("a", "b", "A")) == ("a", "A")
+        @test AutoMerge._find_lowercase_duplicates(("ab", "bb", "aB")) == ("ab", "aB")
+        @test AutoMerge._find_lowercase_duplicates(["ab", "bc"]) === nothing
+        @test AutoMerge._find_lowercase_duplicates(("ab", "bb", "aB", "AB")) == ("ab", "aB")
+    end
+    @testset "meets_file_dir_name_check" begin
+        @test AutoMerge.meets_file_dir_name_check("hi")[1]
+        @test AutoMerge.meets_file_dir_name_check("hi bye")[1]
+        @test AutoMerge.meets_file_dir_name_check("hi.txt")[1]
+        @test AutoMerge.meets_file_dir_name_check("hi.con")[1]
+        @test !AutoMerge.meets_file_dir_name_check("con")[1]
+        @test !AutoMerge.meets_file_dir_name_check("lpt5")[1]
+        @test !AutoMerge.meets_file_dir_name_check("hi.")[1]
+        @test !AutoMerge.meets_file_dir_name_check("hi.txt.")[1]
+        @test !AutoMerge.meets_file_dir_name_check("hi ")[1]
+        @test !AutoMerge.meets_file_dir_name_check("hi:")[1]
+        @test !AutoMerge.meets_file_dir_name_check("hi:bye")[1]
+        @test !AutoMerge.meets_file_dir_name_check("hi?bye")[1]
+        @test !AutoMerge.meets_file_dir_name_check("hi>bye")[1]
+    end
+    @testset "meets_src_names_ok: duplicates" begin
+        @test !AutoMerge.meets_src_names_ok("DOES NOT EXIST")[1]
+        tmp = mktempdir()
+        @test !AutoMerge.meets_src_names_ok(tmp)[1]
+        mkdir(joinpath(tmp, "src"))
+        @test AutoMerge.meets_src_names_ok(tmp)[1]
+        touch(joinpath(tmp, "src", "a"))
+        @test AutoMerge.meets_src_names_ok(tmp)[1]
+
+        if !isdir(joinpath(tmp, "SRC"))
+            mkdir(joinpath(tmp, "src", "A"))
+
+            # dir vs file fails
+            @test !AutoMerge.meets_src_names_ok(tmp)[1]
+            rm(joinpath(tmp, "src", "a"))
+
+            @test AutoMerge.meets_src_names_ok(tmp)[1]
+
+            touch(joinpath(tmp, "src", "A", "b"))
+            @test AutoMerge.meets_src_names_ok(tmp)[1]
+            touch(joinpath(tmp, "src", "b"))
+            # repetition at different levels is OK
+            @test AutoMerge.meets_src_names_ok(tmp)[1]
+
+            touch(joinpath(tmp, "src", "A", "B"))
+            # repetition at the same level is not OK
+            @test !AutoMerge.meets_src_names_ok(tmp)[1]
+        else
+            @warn "Case insensitive filesystem detected, so skipping some `meets_src_files_distinct` checks."
+        end
+    end
+    @testset "meets_src_names_ok: names" begin
+        tmp = mktempdir()
+        mkdir(joinpath(tmp, "src"))
+        @test AutoMerge.meets_src_names_ok(tmp)[1]
+        mkdir(joinpath(tmp, "src", "B"))
+        @test AutoMerge.meets_src_names_ok(tmp)[1]
+        touch(joinpath(tmp, "src", "B", "con"))
+        @test !AutoMerge.meets_src_names_ok(tmp)[1]
+    end
     @testset "pull-requests.jl" begin
         @testset "regexes" begin
             @testset "new_package_title_regex" begin