exthereum / exth_crypto

Cryptographic Functions used by Exthereum
MIT License
22 stars 18 forks source link

recent libsecp25k1 upgrade break Linux install (in a new and fun way) #10

Closed sensay-nelson closed 6 years ago

sensay-nelson commented 6 years ago

libsecp25k1 update appears to break installs on Linux.

commit: https://github.com/exthereum/exth_crypto/commit/daf4b6189ba56a902b3a2c96d4a59102058bd7ed

Error

mix compile
...
Error while loading project :libsecp256k1 at /usr/src/myapp/deps/libsecp256k1
** (MatchError) no match of right hand side value: {"", 128}
    /usr/src/myapp/deps/libsecp256k1/mix.exs:16: Libsecp256k1.Mixfile.project/0

Looking at line 16 in libsecp256k1, it appears to expect a git repository for an unknown reason: https://github.com/exthereum/libsecp256k1/blob/0db4a727e4abdd1c813c88850730c06e3c36fd6c/mix.exs#L16

I'm digging in to see if I can find a workaround or solution, but thought I would comment here for any developers running into this issue.

hayesgm commented 6 years ago

Hmm, trying to upgrade that project to use mix properly must have caused an issue. Should we revert while we investigate?

sensay-nelson commented 6 years ago

If it helps any, this appears to be the offending line upstream:

{filelist, 0} = System.cmd("git", ["ls-files"])

Which appears to be used in an attempt to automatically set the "files" attribute specifically in the package definition:

package: [
       files: Enum.drop(String.split(filelist, "\n"), -1),

According to the docs here: https://hex.pm/docs/publish "files" is optional if the project is using "standard project directories".

:files
A list of files and directories to include in the package. Defaults to standard project directories, so you usually don't need to set this property.

In the example, these appear to be the "standard" directories:

      # These are the default files included in the package
      files: ~w(lib priv .formatter.exs mix.exs README* readme* LICENSE*
                license* CHANGELOG* changelog* src),

I'm thinking that a declarative fuzzy match would work better than a system call to "git list-files". It doesn't seem to be a big list of files in exthereum/libsecp256k1 and I suspect they don't change too often.

I'll let you decide if reverting is necessary. It is broken on new builds at the moment, but it looks like it shouldn't be a big fix upstream.

atoulme commented 6 years ago

Hey, @sensay-nelson thanks for reporting this issue. Yes, I used git ls-files as libsecp256k1 must not add the .so files under priv. There are a number of files that are checked out during the build sequence of the lib, such as the C library under the c_src folder, which I didn't want to see included in the package.

I can change that logic to be very prescriptive, probably by getting the output of git ls-files once, and go from there :)

sensay-nelson commented 6 years ago

Thank you @atoulme . That would be awesome.

atoulme commented 6 years ago

@sensay-nelson please take a look at this PR: https://github.com/exthereum/libsecp256k1/pull/11

hayesgm commented 6 years ago

@sensay-nelson Can you try version 0.1.6, which includes a number of fixes from @atoulme

sensay-nelson commented 6 years ago

@hayesgm this package is only installed in my stack via exthereum/exth_crypto, which I believe skipped the 0.1.6 version.

EDIT: sorry, i see what your saying now, 0.1.6 of this repo. Let me give that a try, just got back from meetings.

sensay-nelson commented 6 years ago

via install of ethereumex 0.3.3, i verified exth_crypto 0.1.6 installs correctly and fixes the issue. Thank you both for your quick turnaround here!

atoulme commented 6 years ago

Thanks for checking @sensay-nelson. Sorry for breaking everything as I learn (hopefully).