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

Fixed temporary #27 (Fixed the way to read basic.c to be used as a library.) #31

Merged
merged 18 commits into from
Sep 20, 2019

Conversation

zacky1972
Copy link
Collaborator

Apply @josevalim advice "use Application.app_dir/2".

This is a just temporary PR. The following error remains:

clang -Ofast -g -ansi -pedantic -femit-all-decls -I/Users/zacky/.asdf/installs/erlang/22.0.7/erts-10.4.4/include -I/usr/local/include -I/usr/include -L/usr/local/lib -L/usr/lib -std=c11 -Wno-unused-function -fPIC -shared -dynamiclib -undefined dynamic_lookup -o /Users/zacky/github/test/_build/dev/lib/pelemay/priv/libnif.so /Users/zacky/github/test/_build/dev/lib/pelemay/native/lib.c

21:33:08.867 [warn]  The on_load function for module Elixir.PelemayNif returned:
{:error, {:bad_lib, 'Function not found \'Elixir.PelemayNif\':map_minus/1'}}

This indicates the old and the generated PelamayNif modules are inconsistent.

@zacky1972 zacky1972 added bug Something isn't working help wanted Extra attention is needed labels Sep 17, 2019
@zacky1972
Copy link
Collaborator Author

Thank you, @muramasa8191, @olafura and @josevalim.

@zacky1972
Copy link
Collaborator Author

I fixed the inconsistent issue by rewriting the generated stub code PelemayNif into the same path to the older stub code definition.

@hisaway, I believe that to fix #28 will bring to fix whole this issues.

@zacky1972 zacky1972 removed the help wanted Extra attention is needed label Sep 17, 2019
@zacky1972
Copy link
Collaborator Author

In this approach, I found that we will meet another future issue that the stub code and the native library conflicts each other when some libraries uses Pelemay at the same time...

To solve it, we should name the stub code and the native library after an independent name.

…h other when some libraries uses Pelemay at the same time
@zacky1972
Copy link
Collaborator Author

@hisaway I've fixed this issue completely!!

I guess to fix #28 is not any longer necessary for this issue.

@zacky1972 zacky1972 removed the bug Something isn't working label Sep 17, 2019
@zacky1972
Copy link
Collaborator Author

Oh, It doesn't work yet...

lib/pelemay.ex Outdated
@@ -40,7 +40,7 @@ defmodule Pelemay do
Db.init()

functions
|> SumMag.map(&Optimizer.replace_expr(&1))
|> SumMag.map(&Optimizer.replace_expr(&1, ("Elixir.PelemayNif" <> (__MODULE__ |> Generator.module_replaced_non())) |> String.to_atom()))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

An alternative expression instead of __MODULE__ that expresses a caller module of the macro is better.
I can't find this expression.

@zacky1972
Copy link
Collaborator Author

At last, It works!!!

However, it may have another issue that may be caused by this:

cbc43fb#r325459075

@zacky1972
Copy link
Collaborator Author

I replaced __MODULE__ into __CALLER__.module, and done some changes.

It works well!!! Huu!

@zacky1972
Copy link
Collaborator Author

Unn, it doesn't work well when Pelemay is installed from git branch...

@zacky1972
Copy link
Collaborator Author

I found the reason why it doesn't work well.

When Pelemay is installed from git or another local path, iex -S mix or other commands doesn't call mix compile.

If iex -S mix runs after mix compile, it works.

Thus, installing by hex requires modification mix.exs to run mix compile before any commands.

@zacky1972 zacky1972 self-assigned this Sep 18, 2019
@zacky1972 zacky1972 added the help wanted Extra attention is needed label Sep 18, 2019
@zacky1972
Copy link
Collaborator Author

I'd like to run elixir-make when Pelemay is installed by deps as follows (in mix.exs) :

  defp deps do
    [
      {:pelemay, git: "https://github.com/zeam-vm/pelemay.git", branch: "muramasa8191-fix-path-for-deps"},
    ]
  end

@zacky1972
Copy link
Collaborator Author

@ConnorRigby said in the Elixir-lang Slack:

elixir-make will be ran at the deps.compile step (which is called for you when you mix compile or mix run
i think your issue is the following: https://github.com/zeam-vm/pelemay/blob/master/mix.exs#L9
should read: [:elixir_make | Mix.compilers() ], instead of having Mix.compilers() first. This will force any c code to be compiled before any elixir code. Which means the nif will be available before the module that uses it is compiled

@zacky1972
Copy link
Collaborator Author

@ConnorRigby Thank you for your comment!

I tried it and I confirmed it works well in the case that Pelemay is alone,
but it doesn't work in the case that another test project is created by mix new test, install Pelemay with mix.exs, compile with mix compile and run iex -S mix.

The test project setting is as follows:

defmodule Test.MixProject do
  use Mix.Project

  def project do
    [
      app: :test,
      version: "0.1.0",
      elixir: "~> 1.9",
      start_permanent: Mix.env() == :prod,
      deps: deps()
    ]
  end

  # Run "mix help compile.app" to learn about applications.
  def application do
    [
      extra_applications: [:logger]
    ]
  end

  # Run "mix help deps" to learn about dependencies.
  defp deps do
    [
      {:pelemay, path: "../pelemay"},
      # {:dep_from_hexpm, "~> 0.3.0"},
      # {:dep_from_git, git: "https://github.com/elixir-lang/my_dep.git", tag: "0.1.0"}
    ]
  end
end
defmodule Test do
  require Pelemay
  import Pelemay

  defpelemay do
    def enum_test list do
      list |> Enum.map(& &1 * 2)
    end
  end

  def do_test do
  	1..1000 |> Enum.to_list |> enum_test()
  end
end

When running iex -S mix with the scriptTest.do_test, then I got the following error:

iex(1)> Test.do_test

10:42:31.274 [warn]  The on_load function for module Elixir.PelemayNifElixirTest returned:
{:error, {:load_failed, 'Failed to load NIF library: \'dlopen(/Users/zacky/github/test/_build/dev/lib/pelemay/priv/libnifelixirtest.so, 2): image not found\''}}

** (UndefinedFunctionError) function PelemayNifElixirTest.map_mult/1 is undefined (module PelemayNifElixirTest is not available)
    (test) PelemayNifElixirTest.map_mult([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, ...])
iex(1)> 

One of this reason is not to build with Makefile by elixir_make.
I can't find where path of Makefile is in the test project and How should I order it to elixir_make.

@zacky1972
Copy link
Collaborator Author

@ConnorRigby I guess it will be solved I create Mix.Tasks.Compile.Pelemay to invoke elixir_make to build native code.

I can't find how to invoke elixir_make to build native code with Makefile.

@zacky1972
Copy link
Collaborator Author

@ConnorRigby said:

okay i think i see the issue now. There are a couple options. You can choose one of the following:

  1. make the user add a pelemay compiler to THEIR mix.exs file. (this is the most common route)
  2. make the user define an alias for the compile task. (this would accomplish the same thing as above, but be less "idiomatic")
  3. in the generate step of defpelemay, just compile it manually there. (this would have the least user intervention, but might get kind of messy)

@zacky1972
Copy link
Collaborator Author

Thank you @ConnorRigby

I understand that the approach 3 is to invoke elixir_make by Pelemay itself.

@ConnorRigby said:

correct. Or some other way of compiling the C code "just in time"

I'd like to choose 1 or 3.
Considering for future, I prefer 3.

@zacky1972
Copy link
Collaborator Author

Ah, I got it! @ConnorRigby
Pelemay can invoke clang directly, instead of elixir_make!!!

@zacky1972 zacky1972 removed the help wanted Extra attention is needed label Sep 19, 2019
@zacky1972 zacky1972 merged commit b58683e into master Sep 20, 2019
@zacky1972 zacky1972 deleted the muramasa8191-fix-path-for-deps branch September 20, 2019 04:04
@zacky1972 zacky1972 mentioned this pull request Sep 21, 2019
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.

None yet

2 participants