Skip to content
This repository has been archived by the owner on Aug 23, 2018. It is now read-only.

Make search for dirname prefix case insensitive #211

Closed
wants to merge 1 commit into from
Closed

Make search for dirname prefix case insensitive #211

wants to merge 1 commit into from

Conversation

Janiczek
Copy link

Fixes the problem when package maintainer's GitHub account is
uppercase and the package has repository URL with lowercase.
Then the package couldn't be downloaded.

Example:
on Github, Janiczek/elm-markov
in elm-package.json, https://github.com/janiczek/elm-markov.git
(note janiczek vs Janiczek)

Steps to reproduce the problem that happens before this fix:

$ mkdir test-project
$ cd test-project
$ elm-package install janiczek/elm-markov --yes
Downloading elm-community/elm-list-extra
Downloading elm-lang/core
Downloading janiczek/elm-markov

Error: Could not download source code successfully.

(because directory from the zipball is Janiczek-... and it's searching for janiczek as a prefix.)

Technically could have been done with Data.Char and Data.List instead of Data.Text, I don't know what's preferred.

Fixes the problem when package maintainer's GitHub account is
uppercase and the package has repository URL with lowercase.
Then the package couldn't be downloaded.

Example:
on Github, Janiczek/elm-markov
in elm-package.json, https://github.com/janiczek/elm-markov.git
(note janiczek vs Janiczek)
@evancz
Copy link
Contributor

evancz commented May 23, 2016

Can you explain what you did? Did you publish the same library with different capitalization? Please explain exactly.

@Janiczek
Copy link
Author

Janiczek commented May 23, 2016

I published elm-markov under janiczek/elm-markov, not Janiczek/elm-markov (see repository here).

Zipball downloaded by elm-package has a directory named Janiczek-elm-markov-23a5cde, but elm-package looks for a directory starting with janiczek. Doesn't find, errors out.

@karls
Copy link

karls commented Sep 26, 2016

Hi all,

I have just hit this problem when trying to install https://github.com/wernerdegroot/listzipper/tree/1.0.2.

Steps to reproduce are the same as @Janiczek outlined:

$ mkdir test
$ cd test
$ elm-package install wernerdegroot/listzipper --yes
Starting downloads...

  ✗ wernerdegroot/listzipper 1.0.2
  ● elm-lang/html 1.1.0
  ● elm-lang/virtual-dom 1.1.1
  ● elm-lang/core 4.0.5
Error: Problem when downloading the wernerdegroot/listzipper 1.0.2 code.

As @Janiczek pointed out, the reason is case sensitivity. The repository name (on Github) for the package I was trying to install is ListZipper, which means that the zipball that gets downloaded is named something like wernerdegroot-ListZipper-1.0.2-0-g4f5b481.zip.

However, elm-package performs the lookup for the downloaded zipball under the incorrect assumption that the username and repository are both lowercase. The relevant line is here:

case List.find (List.isPrefixOf (user ++ "-" ++ project)) files of
.

I have also opened a PR #241 which fixes this bug for the current master branch.

@karls
Copy link

karls commented Sep 27, 2016

Actually, it was pointed out to me that installing NoRedInk packages works fine. I was wrong to say that the username is assumed to be lowercase. The point about the package/repository name remains.

I have also updated my pull request to reflect that -- the comparison will be between lowercased zipball file name and lowercased package name.

Let me know if I can do anything else.

@evancz
Copy link
Contributor

evancz commented Jul 10, 2017

In the latest code I rename the files directly, avoiding the old process that downloaded with some weird name and tries to find it and move it. That means this issue should be avoided in a more reliable way now.

@evancz evancz closed this Jul 10, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants