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

Fix elixir 1.0.x compatibility #307

Merged
merged 2 commits into from
Nov 1, 2016
Merged

Fix elixir 1.0.x compatibility #307

merged 2 commits into from
Nov 1, 2016

Conversation

mneudert
Copy link
Contributor

After the last changes the backwards compatibility with Elixir 1.0.x versions has been accidentally broken.

Changing MapSet.new to the already existing Hex.Set.new wrapper should fix that.

This was the actual error message I encountered:

** (UndefinedFunctionError) undefined function: MapSet.new/1 (module MapSet is not available)
    MapSet.new(["CHANGES.md", "LICENSE", "mix.exs", "rebar.config", "README.md"])
    (hex) lib/hex/scm.ex:142: Hex.SCM.guess_build_tools/1
    (hex) lib/hex/scm.ex:111: Hex.SCM.checkout/1
    (mix) lib/mix/dep/fetcher.ex:64: Mix.Dep.Fetcher.do_fetch/3
    (mix) lib/mix/dep/converger.ex:154: Mix.Dep.Converger.all/8
    (mix) lib/mix/dep/converger.ex:163: Mix.Dep.Converger.all/8
    (mix) lib/mix/dep/converger.ex:47: Mix.Dep.Converger.converge/4
    (mix) lib/mix/dep/fetcher.ex:16: Mix.Dep.Fetcher.all/3

@ericmj
Copy link
Member

ericmj commented Oct 30, 2016

Thank you @mneudert.

I think the regression tests missed this because this code is only hit for old packages and it's tricky for us to test packages that the server no longer accepts. Would you mind making the function public and adding a unit test for it?

@mneudert
Copy link
Contributor Author

mneudert commented Nov 1, 2016

Does the test what you were looking for?

(It already catched the Enum.into/2 I missed as the old HashSet does not provide a new/1...)

@@ -0,0 +1,15 @@
defmodule Hex.CryptoTest do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hex.SCMTest :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No one has ever seen that 🙄

@ericmj
Copy link
Member

ericmj commented Nov 1, 2016

Looks great. Will merge when CI passes. ❤️

@ericmj ericmj merged commit f802a1e into hexpm:master Nov 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants