From c681c6e8e592f9069f5ca2f670252d44527addae Mon Sep 17 00:00:00 2001 From: Matthew Flatt Date: Wed, 16 Jul 2014 15:20:14 +0100 Subject: [PATCH] raco pkg install: reject overlapping package directories For example, if you accidentally type `raco pkg install` in your "racket" directory, you'd rather have an error rather than competing module paths that reach all libraries. --- .../racket-test/tests/pkg/tests-conflicts.rkt | 14 +-- .../racket-test/tests/pkg/tests-install.rkt | 101 +++++++++++------- .../racket-test/tests/pkg/tests-scope.rkt | 22 ++-- .../racket-test/tests/pkg/tests-update.rkt | 49 +++++---- .../racket-test/tests/pkg/util.rkt | 2 +- racket/collects/pkg/lib.rkt | 57 ++++++++++ 6 files changed, 168 insertions(+), 77 deletions(-) diff --git a/pkgs/racket-pkgs/racket-test/tests/pkg/tests-conflicts.rkt b/pkgs/racket-pkgs/racket-test/tests/pkg/tests-conflicts.rkt index 125f83c608..aa2c2b4e41 100644 --- a/pkgs/racket-pkgs/racket-test/tests/pkg/tests-conflicts.rkt +++ b/pkgs/racket-pkgs/racket-test/tests/pkg/tests-conflicts.rkt @@ -57,15 +57,15 @@ $ "test -f test-pkgs/pkg-test3.zip" $ "raco pkg install test-pkgs/pkg-test3.zip" =exit> 1) + (define tmp-dir (path->directory-path (make-temporary-file "pkg~a" 'directory))) (shelly-wind - $ "cp -r test-pkgs/pkg-test1 test-pkgs/pkg-test1-linking" - (shelly-install* "conflicts are caught, even with a link" - "--link test-pkgs/pkg-test1-linking" - "pkg-test1-linking" - $ "test -f test-pkgs/pkg-test1-conflict.zip" - $ "raco pkg install test-pkgs/pkg-test1-conflict.zip" =exit> 1) + $ (~a "cp -r test-pkgs/pkg-test1 " tmp-dir"pkg-test1-linking") + $ (~a "raco pkg install --link " tmp-dir"pkg-test1-linking") + $ "test -f test-pkgs/pkg-test1-conflict.zip" + $ "raco pkg install test-pkgs/pkg-test1-conflict.zip" =exit> 1 + $ "raco pkg remove pkg-test1-linking" (finally - $ "rm -fr test-pkgs/pkg-test1-linking")) + (delete-directory/files tmp-dir))) (shelly-install "conflicts can be forced" "test-pkgs/pkg-test1.zip" $ "racket -e '(require pkg-test1/conflict)'" =exit> 42 diff --git a/pkgs/racket-pkgs/racket-test/tests/pkg/tests-install.rkt b/pkgs/racket-pkgs/racket-test/tests/pkg/tests-install.rkt index d5950737f4..0a8a7999ac 100644 --- a/pkgs/racket-pkgs/racket-test/tests/pkg/tests-install.rkt +++ b/pkgs/racket-pkgs/racket-test/tests/pkg/tests-install.rkt @@ -8,10 +8,12 @@ racket/runtime-path racket/path racket/list + racket/format file/zip file/unzip net/url pkg/util + setup/dirs "shelly.rkt" "util.rkt") @@ -31,12 +33,8 @@ (shelly-install* "local package (zip, single-collection)" "test-pkgs/pkg-test1.zip test-pkgs/pkg-test3.zip" "pkg-test1 pkg-test3") - (shelly-install "local package (dir)" (string-append - "--copy " - (url->string (path->url (path->complete-path "test-pkgs/pkg-test1"))))) - (shelly-install "local package (file://dir)" (string-append - "--copy " - (url->string (path->url (path->complete-path "test-pkgs/pkg-test1"))))) + (shelly-install "local package (dir)" (url->string (path->url (path->complete-path "test-pkgs/pkg-test1")))) + (shelly-install "local package (file://dir)" (url->string (path->url (path->complete-path "test-pkgs/pkg-test1")))) ;; Check ".zip" file with extra directory layer: (let ([dir (make-temporary-file "zip~a" 'directory)] @@ -117,56 +115,79 @@ (shelly-case "local directory fails when not there" - $ "raco pkg install test-pkgs/pkg-test1-not-there/" =exit> 1) + $ "raco pkg install --copy test-pkgs/pkg-test1-not-there/" =exit> 1) + + (shelly-case + "directory fails due to path overlap" + $ "raco pkg install test-pkgs/pkg-test1" + =exit> 1 + =stderr> #rx"overlap" + $ (~a "raco pkg install " (find-collects-dir)) + =exit> 1 + =stderr> #rx"overlap.*collection" + $ (~a "raco pkg install " (collection-path "tests")) + =exit> 1 + =stderr> #rx"overlap.*package") + + (define tmp-dir (path->directory-path (make-temporary-file "pkg~a" 'directory))) + $ (~a "cp -r test-pkgs/pkg-test1 "tmp-dir"pkg-test1") + $ (~a "cp -r test-pkgs/pkg-test3 "tmp-dir"pkg-test3") + + (shelly-case + "directory fails due to path overlap" + $ (~a "raco pkg install "tmp-dir" "tmp-dir"pkg-test1") + =exit> 1 + =stderr> #rx"overlap") (shelly-install "local package (directory)" "test-pkgs/pkg-test1/" $ "racket -e '(require pkg-test1)'") (shelly-install* "local package (single-collection directory)" - "test-pkgs/pkg-test1/ test-pkgs/pkg-test3/" + (~a tmp-dir"pkg-test1/ "tmp-dir"pkg-test3/") "pkg-test1 pkg-test3" $ "racket -e '(require pkg-test3)'") - - (shelly-install "redundant packages ignored" "test-pkgs/pkg-test1/ test-pkgs/pkg-test1/" + + (shelly-install "redundant packages ignored" + (~a tmp-dir"pkg-test1/ "tmp-dir"pkg-test1/") $ "racket -e '(require pkg-test1)'") + (shelly-case "conflicting package names disallowed" - $ "raco pkg install test-pkgs/pkg-test1/ test-pkgs/pkg-test1.zip" =exit> 1) + $ "raco pkg install --copy test-pkgs/pkg-test1/ test-pkgs/pkg-test1.zip" =exit> 1) + + $ (~a "cp -r "tmp-dir"pkg-test1 "tmp-dir"pkg-test1-linking") + $ (~a "cp -r test-pkgs/pkg-test1-staging "tmp-dir"pkg-test1-staging") (with-fake-root (shelly-case "linking local directory" - (shelly-wind - $ "cp -r test-pkgs/pkg-test1 test-pkgs/pkg-test1-linking" - $ "racket -e '(require pkg-test1)'" =exit> 1 - $ "raco pkg install --link test-pkgs/pkg-test1-linking" - $ "racket -e '(require pkg-test1)'" - $ "racket -e '(require pkg-test1/a)'" =exit> 1 - $ "racket -e '(require pkg/lib)' -e '(path->pkg \"test-pkgs/pkg-test1-linking\")'" =stdout> "\"pkg-test1-linking\"\n" - $ "racket -e '(require pkg/lib)' -e '(path->pkg \"test-pkgs/pkg-test1-linking/README\")'" =stdout> "\"pkg-test1-linking\"\n" - $ "racket -e '(require pkg/lib)' -e '(path->pkg \"test-pkgs\")'" =stdout> "\"racket-test\"\n" - $ "racket -e '(require pkg/lib)' -e '(path->pkg (collection-file-path \"main.rkt\" \"racket\"))'" =stdout> "#f\n" - $ "cp test-pkgs/pkg-test1-staging/a.rkt test-pkgs/pkg-test1-linking/pkg-test1/a.rkt" - $ "racket -e '(require pkg-test1/a)'" - $ "rm -f test-pkgs/pkg-test1-linking/pkg-test1/a.rkt" - $ "racket -e '(require pkg-test1/a)'" =exit> 1 - $ "raco pkg remove pkg-test1-linking" - $ "racket -e '(require pkg-test1)'" =exit> 1 - (finally - $ "rm -r test-pkgs/pkg-test1-linking")))) + $ "racket -e '(require pkg-test1)'" =exit> 1 + $ (~a "raco pkg install --link "tmp-dir"pkg-test1-linking") + $ "racket -e '(require pkg-test1)'" + $ "racket -e '(require pkg-test1/a)'" =exit> 1 + $ (~a "racket -e '(require pkg/lib)' -e '(path->pkg \""tmp-dir"pkg-test1-linking\")'") =stdout> "\"pkg-test1-linking\"\n" + $ (~a "racket -e '(require pkg/lib)' -e '(path->pkg \""tmp-dir"pkg-test1-linking/README\")'") =stdout> "\"pkg-test1-linking\"\n" + $ "racket -e '(require pkg/lib)' -e '(path->pkg \"test-pkgs\")'" =stdout> "\"racket-test\"\n" + $ "racket -e '(require pkg/lib)' -e '(path->pkg (collection-file-path \"main.rkt\" \"racket\"))'" =stdout> "#f\n" + $ (~a "cp "tmp-dir"pkg-test1-staging/a.rkt "tmp-dir"pkg-test1-linking/pkg-test1/a.rkt") + $ "racket -e '(require pkg-test1/a)'" + $ (~a "rm -f "tmp-dir"pkg-test1-linking/pkg-test1/a.rkt") + $ "racket -e '(require pkg-test1/a)'" =exit> 1 + $ "raco pkg remove pkg-test1-linking" + $ "racket -e '(require pkg-test1)'" =exit> 1)) + + $ (~a "cp -r "tmp-dir"pkg-test3 "tmp-dir"pkg-test3-linking") (with-fake-root (shelly-case "linking local directory, single-collection" - (shelly-wind - $ "cp -r test-pkgs/pkg-test3 test-pkgs/pkg-test3-linking" - $ "racket -e '(require pkg-test3)'" =exit> 1 - $ "raco pkg install --link test-pkgs/pkg-test1 test-pkgs/pkg-test3-linking" - $ "racket -e '(require pkg-test3-linking)'" - $ "racket -e '(require pkg-test3)'" =exit> 1 - $ "raco pkg remove pkg-test1 pkg-test3-linking" - $ "racket -e '(require pkg-test3-linking)'" =exit> 1 - (finally - $ "rm -r test-pkgs/pkg-test3-linking")))) + $ "racket -e '(require pkg-test3)'" =exit> 1 + $ (~a "raco pkg install --link "tmp-dir"pkg-test1 "tmp-dir"pkg-test3-linking") + $ "racket -e '(require pkg-test3-linking)'" + $ "racket -e '(require pkg-test3)'" =exit> 1 + $ "raco pkg remove pkg-test1 pkg-test3-linking" + $ "racket -e '(require pkg-test3-linking)'" =exit> 1)) + + (delete-directory/files tmp-dir) (with-fake-root (shelly-case @@ -178,7 +199,7 @@ (shelly-case "implicitly single-collection" $ "racket -e '(require pkg-c/c)'" =exit> 1 - $ "raco pkg install --link test-pkgs/pkg-c" + $ "raco pkg install --copy test-pkgs/pkg-c" $ "racket -e '(require pkg-c/c)'" =stdout> "'c\n")) (with-fake-root diff --git a/pkgs/racket-pkgs/racket-test/tests/pkg/tests-scope.rkt b/pkgs/racket-pkgs/racket-test/tests/pkg/tests-scope.rkt index fdfe7181a0..95ea117d03 100644 --- a/pkgs/racket-pkgs/racket-test/tests/pkg/tests-scope.rkt +++ b/pkgs/racket-pkgs/racket-test/tests/pkg/tests-scope.rkt @@ -31,25 +31,25 @@ (shelly-case "conflict due to installations in different scopes: installation-wide first" - $ "raco pkg install -i test-pkgs/pkg-test1" + $ "raco pkg install -i --copy test-pkgs/pkg-test1" $ "racket -l pkg-test1" =stdout> #rx"main loaded" - $ "raco pkg install -u test-pkgs/pkg-test1" =exit> 1 =stderr> #rx"installed in a wider scope" - $ "raco pkg install -u --name base test-pkgs/pkg-test1" =exit> 1 =stderr> #rx"installed in a wider scope" + $ "raco pkg install -u --copy test-pkgs/pkg-test1" =exit> 1 =stderr> #rx"installed in a wider scope" + $ "raco pkg install -u --name base --copy test-pkgs/pkg-test1" =exit> 1 =stderr> #rx"installed in a wider scope" $ "raco pkg remove pkg-test1" =stdout> #rx"Inferred package scope: installation") (shelly-case "conflict due to installations in different scopes: user-specific first" - $ "raco pkg install -u test-pkgs/pkg-test1" + $ "raco pkg install -u --copy test-pkgs/pkg-test1" $ "racket -l pkg-test1" =stdout> #rx"main loaded" - $ "raco pkg install -i test-pkgs/pkg-test1" =exit> 1 =stderr> #rx"packages in different scopes conflict" + $ "raco pkg install -i --copy test-pkgs/pkg-test1" =exit> 1 =stderr> #rx"packages in different scopes conflict" $ "raco pkg remove pkg-test1" =stdout> "Removing pkg-test1\n") (shelly-case "override conflist, installation first" - $ "raco pkg install -i test-pkgs/pkg-test1" + $ "raco pkg install -i --copy test-pkgs/pkg-test1" $ "racket -l racket/base -l pkg-test1/number -e '(number)'" =stdout> "1\n" - $ "raco pkg install -u --name pkg-test1 test-pkgs/pkg-test1-v2" =exit> 1 - $ "raco pkg install --force -u --name pkg-test1 test-pkgs/pkg-test1-v2" + $ "raco pkg install -u --name pkg-test1 --copy test-pkgs/pkg-test1-v2" =exit> 1 + $ "raco pkg install --force -u --name pkg-test1 --copy test-pkgs/pkg-test1-v2" $ "racket -l racket/base -l pkg-test1/number -e '(number)'" =stdout> "2\n" $ "raco pkg remove pkg-test1" =stdout> "Removing pkg-test1\n" $ "racket -l racket/base -l pkg-test1/number -e '(number)'" =stdout> "1\n" @@ -57,10 +57,10 @@ (shelly-case "override conflist, user first" - $ "raco pkg install -u test-pkgs/pkg-test1" + $ "raco pkg install -u --copy test-pkgs/pkg-test1" $ "racket -l racket/base -l pkg-test1/number -e '(number)'" =stdout> "1\n" - $ "raco pkg install -i --name pkg-test1 test-pkgs/pkg-test1-v2" =exit> 1 - $ "raco pkg install --force -i --name pkg-test1 test-pkgs/pkg-test1-v2" + $ "raco pkg install -i --name pkg-test1 --copy test-pkgs/pkg-test1-v2" =exit> 1 + $ "raco pkg install --force -i --name pkg-test1 --copy test-pkgs/pkg-test1-v2" $ "racket -l racket/base -l pkg-test1/number -e '(number)'" =stdout> "1\n" $ "raco pkg remove pkg-test1" =stdout> "Removing pkg-test1\n" $ "racket -l racket/base -l pkg-test1/number -e '(number)'" =stdout> "2\n" diff --git a/pkgs/racket-pkgs/racket-test/tests/pkg/tests-update.rkt b/pkgs/racket-pkgs/racket-test/tests/pkg/tests-update.rkt index eff2734166..a29c806a55 100644 --- a/pkgs/racket-pkgs/racket-test/tests/pkg/tests-update.rkt +++ b/pkgs/racket-pkgs/racket-test/tests/pkg/tests-update.rkt @@ -8,6 +8,7 @@ racket/runtime-path racket/path racket/list + racket/format pkg/util "shelly.rkt" "util.rkt") @@ -26,9 +27,16 @@ (shelly-install "local packages can't be updated (file)" "test-pkgs/pkg-test1.zip" $ "raco pkg update pkg-test1" =exit> 1) - (shelly-install "local packages can't be updated (directory)" - "test-pkgs/pkg-test1/" - $ "raco pkg update pkg-test1" =exit> 1) + + (define tmp-dir (path->directory-path (make-temporary-file "pkg~a" 'directory))) + (shelly-wind + $ (~a "cp -r test-pkgs/pkg-test1 " tmp-dir"pkg-test1") + (shelly-install "local packages can't be updated (directory)" + (~a tmp-dir"pkg-test1/") + $ "raco pkg update pkg-test1" =exit> 1) + (finally + (delete-directory/files tmp-dir))) + (shelly-wind $ "mkdir -p test-pkgs/update-test" $ "cp -f test-pkgs/pkg-test1-v2.zip test-pkgs/update-test/pkg-test1.zip" @@ -45,21 +53,26 @@ $ "racket -e '(require pkg-test1/update)'" =exit> 42 $ "raco pkg update --name pkg-test1 test-pkgs/pkg-test1-v2.zip" $ "racket -e '(require pkg-test1/update)'" =exit> 43) - (shelly-install "packages can be replaced with local packages (directory)" - "test-pkgs/pkg-test1.zip" - $ "racket -e '(require pkg-test1/update)'" =exit> 42 - $ "raco pkg update --name pkg-test1 test-pkgs/pkg-test1-v2" - $ "racket -e '(require pkg-test1/update)'" =exit> 43) - (shelly-install "replacement checksum can be checked" - "test-pkgs/pkg-test1.zip" - $ "raco pkg update test-pkgs/pkg-test1.zip" =stdout> "No updates available\n") - (shelly-install "checksum can be supplied for local directory" - "test-pkgs/pkg-test1.zip" - $ "racket -e '(require pkg-test1/update)'" =exit> 42 - $ "raco pkg update --name pkg-test1 --checksum abcdef test-pkgs/pkg-test1-v2" - $ "racket -e '(require pkg-test1/update)'" =exit> 43 - $ "raco pkg show" =stdout> #rx"abcdef" - $ "raco pkg update --name pkg-test1 --checksum abcdef test-pkgs/pkg-test1-v2" =stdout> "No updates available\n") + (define tmp2-dir (path->directory-path (make-temporary-file "pkg~a" 'directory))) + (shelly-wind + $ (~a "cp -r test-pkgs/pkg-test1-v2 " tmp2-dir"pkg-test1-v2") + (shelly-install "packages can be replaced with local packages (directory)" + "test-pkgs/pkg-test1.zip" + $ "racket -e '(require pkg-test1/update)'" =exit> 42 + $ (~a "raco pkg update --name pkg-test1 "tmp2-dir"pkg-test1-v2") + $ "racket -e '(require pkg-test1/update)'" =exit> 43) + (shelly-install "replacement checksum can be checked" + "test-pkgs/pkg-test1.zip" + $ "raco pkg update test-pkgs/pkg-test1.zip" =stdout> "No updates available\n") + (shelly-install "checksum can be supplied for local directory" + "test-pkgs/pkg-test1.zip" + $ "racket -e '(require pkg-test1/update)'" =exit> 42 + $ (~a "raco pkg update --name pkg-test1 --checksum abcdef "tmp2-dir"pkg-test1-v2") + $ "racket -e '(require pkg-test1/update)'" =exit> 43 + $ "raco pkg show" =stdout> #rx"abcdef" + $ (~a "raco pkg update --name pkg-test1 --checksum abcdef "tmp2-dir"pkg-test1-v2") =stdout> "No updates available\n") + (finally + (delete-directory/files tmp2-dir))) (shelly-wind $ "mkdir -p test-pkgs/update-test" diff --git a/pkgs/racket-pkgs/racket-test/tests/pkg/util.rkt b/pkgs/racket-pkgs/racket-test/tests/pkg/util.rkt index fd071d7327..e12b533f97 100644 --- a/pkgs/racket-pkgs/racket-test/tests/pkg/util.rkt +++ b/pkgs/racket-pkgs/racket-test/tests/pkg/util.rkt @@ -173,7 +173,7 @@ (format "Test installation of ~a" message) pre ... $ "racket -e '(require pkg-test1)'" =exit> 1 - $ (format "raco pkg install ~a" pkg) + $ (format "raco pkg install --copy ~a" pkg) $ "racket -e '(require pkg-test1)'" more ... $ (format "raco pkg remove ~a" rm-pkg) diff --git a/racket/collects/pkg/lib.rkt b/racket/collects/pkg/lib.rkt index 58329c0991..27bd4ac6b9 100644 --- a/racket/collects/pkg/lib.rkt +++ b/racket/collects/pkg/lib.rkt @@ -1007,6 +1007,55 @@ ;; This file would be redundant, so drop it (delete-file pkg-file))))) +(define (disallow-package-path-overlaps pkg-name + pkg-path + path-pkg-cache + simultaneous-installs) + (define simple-pkg-path (simple-form-path pkg-path)) + (define (one-in-the-other? p1 p2) + (define pe (explode-path p1)) + (define e (explode-path p2)) + (if ((length e) . < . (length pe)) + (equal? (take pe (length e)) e) + (equal? (take e (length pe)) pe))) + ;; Check collects: + (for ([c (in-list (current-library-collection-paths))]) + (when (one-in-the-other? simple-pkg-path + (simple-form-path c)) + (pkg-error (~a "cannot link a directory that overlaps with a collection path\n" + " collection path: ~a\n" + " link path: ~a\n" + " as package: ~a") + c + pkg-path + pkg-name))) + ;; Check installed packages: + (for ([f (in-directory simple-pkg-path)]) + (define found-pkg (path->pkg f #:cache path-pkg-cache)) + (when (and found-pkg + (not (equal? found-pkg pkg-name))) + (pkg-error (~a "cannot link a directory that overlaps with existing packages\n" + " existing package: ~a\n" + " overlapping path: ~a\n" + " a package: ~a") + found-pkg + f + pkg-name))) + ;; Check simultaneous installs: + (for ([(other-pkg other-dir) (in-hash simultaneous-installs)]) + (unless (equal? other-pkg pkg-name) + (when (one-in-the-other? simple-pkg-path + (simple-form-path other-dir)) + (pkg-error (~a "cannot link directories that overlap for different packages\n" + " package: ~a\n" + " path: ~a\n" + " overlapping package: ~a\n" + " overlapping path: ~a") + pkg-name + pkg-path + other-pkg + other-dir))))) + ;; Downloads a package (if needed) and unpacks it (if needed) into a ;; temporary directory. (define (stage-package/info pkg @@ -1518,6 +1567,14 @@ (define simultaneous-installs (for/hash ([i (in-list infos)]) (values (install-info-name i) (install-info-directory i)))) + + (when (and (pair? orig-pkg) + (or (eq? (car orig-pkg) 'link) + (eq? (car orig-pkg) 'static-link))) + (disallow-package-path-overlaps pkg-name + pkg-dir + path-pkg-cache + simultaneous-installs)) (cond [(and (not updating?) (hash-ref all-db pkg-name #f)