Skip to content
This repository has been archived by the owner on Jul 19, 2023. It is now read-only.

fix path problems which lead to new projects not running #26

Merged
merged 2 commits into from
Jul 22, 2019

Conversation

ejc123
Copy link
Contributor

@ejc123 ejc123 commented Jul 18, 2019

This is a potential fix for #25 and a similar bug in scenic. I have tested it with a fresh project generated from mix scenic.new.example as well as mix scenic.new and it works with both MIX_ENV=prod or MIX_ENV=dev, first time and further times.

Makefile Outdated
@@ -49,4 +51,4 @@ priv/$(MIX_ENV)/scenic_driver_glfw: $(SRCS)
clean:
$(RM) -r priv/dev
Copy link

Choose a reason for hiding this comment

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

Why you are not changing priv with ${RPEFIX} in clean target? Also why instead of using ${MIX_ENV} we have remove call for every environment? Also what if we have a custom ${MIX_ENV}? From what I can see it would be not cleaned properly …

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you. I hadn't even thought of doing that. Changed committed.

@boydm
Copy link
Collaborator

boydm commented Jul 19, 2019

Are you able to get mix test to run? It is failing to find the executable in the test environment for me...

@boydm
Copy link
Collaborator

boydm commented Jul 19, 2019

Also... I really appreciate the help with this!

Copy link

@Eiji7 Eiji7 left a comment

Choose a reason for hiding this comment

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

Personally I would change all tabulations to 2 spaces and store scenic_driver_glfw under PROJECT_NAME variable, but it's not something as important as making it working. I have tested it and it works for me as well, so for me it could be approved.

@Eiji7
Copy link

Eiji7 commented Jul 20, 2019

@boydm Just tried it and mix test works for me (for both example projects and this project standalone). Can you please copy your whole compilation output?

@boydm
Copy link
Collaborator

boydm commented Jul 20, 2019

@Eiji7

[scenic_driver_glfw (ejc123-master)]$ mix test
mkdir -p /Users/boyd/git/scenic/split/scenic_driver_glfw/_build/test/lib/scenic_driver_glfw/ebin/../priv/test
cc -O3 -std=c99 `pkg-config --static --cflags glfw3 glew` -fPIC -o /Users/boyd/git/scenic/split/scenic_driver_glfw/_build/test/lib/scenic_driver_glfw/ebin/../priv/test/scenic_driver_glfw c_src/main.c c_src/comms.c c_src/nanovg/nanovg.c c_src/utils.c c_src/render_script.c c_src/tx.c `pkg-config --static --libs glfw3 glew` -framework Cocoa -framework OpenGL -Wno-deprecated
Compiling 7 files (.ex)
Generated scenic_driver_glfw app
sh: /Users/boyd/git/scenic/split/scenic_driver_glfw/_build/test/lib/scenic_driver_glfw/priv/test/scenic_driver_glfw: No such file or directory
sh: line 0: exec: /Users/boyd/git/scenic/split/scenic_driver_glfw/_build/test/lib/scenic_driver_glfw/priv/test/scenic_driver_glfw: cannot execute: No such file or directory


  1) test Integration style test (Scenic.Driver.GlfwTest)
     test/glfw_test.exs:35
     Assertion with == failed
     code:  assert Glfw.Cache.load_static_texture(@parrot_hash, state.port()) == true
     left:  nil
     right: true
     stacktrace:
       test/glfw_test.exs:69: (test)

Finished in 1.7 seconds
1 test, 1 failure

Randomized with seed 320809
[scenic_driver_glfw (ejc123-master)]$

@Eiji7
Copy link

Eiji7 commented Jul 20, 2019

@boydm Of course except parent directory and compilation flags (I have no MacOS) all paths are exactly same. After Generated scenic_driver_glfw app I can see that tests finished successfully (I saw window without problem). For sure here is my output:

$ mix test
mkdir -p /home/eiji/scenic_driver_glfw/_build/test/lib/scenic_driver_glfw/ebin/../priv/test
cc -O3 -std=c99 `pkg-config --static --cflags glfw3 glew` -fPIC -o /home/eiji/scenic_driver_glfw/_build/test/lib/scenic_driver_glfw/ebin/../priv/test/scenic_driver_glfw c_src/main.c c_src/comms.c c_src/nanovg/nanovg.c c_src/utils.c c_src/render_script.c c_src/tx.c `pkg-config --static --libs glfw3 glew` -lGL -lm -lrt
.

Finished in 3.8 seconds
1 test, 0 failures

Randomized with seed 681971

@axelson
Copy link

axelson commented Jul 20, 2019

@boydm Interesting, I'm only able to get that error intermittently, but only on the master branch so far (instead of this PR), not sure how to reproduce it. I've been able to get the error on both Mac OS and Linux. What version of Elixir and Erlang are you using? I've tested with Elixir 1.8.1-otp-21 (w Erlang 21.2.4) and Elixir 1.9.1-otp-22 (w Erlang 22.0.7).

@boydm
Copy link
Collaborator

boydm commented Jul 20, 2019

@axelson
OTP 22 and elixir 1.9.0

I'll update to 1.9.1 and see what happens...

@boydm
Copy link
Collaborator

boydm commented Jul 20, 2019

Same with 1.9.1.

@Eiji7
Copy link

Eiji7 commented Jul 20, 2019

Hmm … I have tried different Erlang and Elixir versions. For now I have:

$ asdf current
elixir         1.9.1-otp-22 (set by)
erlang         22.0.7   (set by)

I still can't reproduce this problem. @axelson said that he got error also on Linux, but tried lots of times and I'm unable to reproduce it. I have cloned axelson/scenic_driver_glfw and even changed in mix.exs dependency scenic to fetch it from github i.e. github: "boydm/scenic".

Am I doing something wrong?

@axelson
Copy link

axelson commented Jul 21, 2019

I'm only able to reproduce the error on master with rm -rf _build; rm -rf priv; git checkout priv; mix test. But it's not actually the same error since on master gcc is outputting to priv/test/scenic_driver_glfw whereas on this branch it outputs to the full build directory.

The only thing I can think of is if Boyd's ebin folder is a symlink because otherwise the cc invocation is putting the binary into /Users/boyd/git/scenic/split/scenic_driver_glfw/_build/test/lib/scenic_driver_glfw/ebin/../priv/test/scenic_driver_glfw, which should match the path /Users/boyd/git/scenic/split/scenic_driver_glfw/_build/test/lib/scenic_driver_glfw/priv/test/scenic_driver_glfw

@boydm do you still have errors if you rm -rf _build deps.

The other weird thing I've noticed is that if I cp -r _build/test/lib/scenic_driver_glfw/priv/test priv then I will get make: Nothing to be done for 'all'., when I would expect that to not matter.

@Eiji7
Copy link

Eiji7 commented Jul 21, 2019

@axelson Sorry I'm just lost or did something amazing accidentally 😄

Your example:

rm -rf _build; rm -rf priv; git checkout priv; mix test

gives me exactly same results i.e. tests works. To make sure I have tried few times.

But it's not actually the same error since on master gcc is outputting to priv/test/scenic_driver_glfw whereas on this branch it outputs to the full build directory.

Sorry, I did not get this, because:

# master
$ git clone https://github.com/ejc123/scenic_driver_glfw.git
$ mix deps.get
$ mix test
cc -O3 -std=c99 `pkg-config --static --cflags glfw3 glew` -fPIC -o /home/eiji/scenic_driver_glfw/_build/test/lib/scenic_driver_glfw/ebin/../priv/test/scenic_driver_glfw c_src/main.c c_src/comms.c c_src/nanovg/nanovg.c c_src/utils.c c_src/render_script.c c_src/tx.c `pkg-config --static --libs glfw3 glew` -lGL -lm -lrt
# priv
$ rm -rf _build; rm -rf priv; git checkout priv; mix test
cc -O3 -std=c99 `pkg-config --static --cflags glfw3 glew` -fPIC -o /home/eiji/scenic_driver_glfw/_build/test/lib/scenic_driver_glfw/ebin/../priv/test/scenic_driver_glfw c_src/main.c c_src/comms.c c_src/nanovg/nanovg.c c_src/utils.c c_src/render_script.c c_src/tx.c `pkg-config --static --libs glfw3 glew` -lGL -lm -lrt

As you can see on both branches gcc gives me full build directory … It's probably why I can't reproduce this problem, right?

@axelson
Copy link

axelson commented Jul 21, 2019

Sorry, by master I mean commit 4e5b0d9 from https://github.com/boydm/scenic_driver_glfw

@Eiji7
Copy link

Eiji7 commented Jul 21, 2019

@axelson ok, got it!

Sorry, I though that we are talking about this PR only which points to your repository 😄

On "master" I have now:

mkdir -p priv/test
cc -O3 -std=c99 `pkg-config --static --cflags glfw3 glew` -fPIC -o priv/test/scenic_driver_glfw c_src/main.c c_src/comms.c c_src/nanovg/nanovg.c c_src/utils.c c_src/render_script.c c_src/tx.c `pkg-config --static --libs glfw3 glew` -lGL -lm -lrt
Compiling 7 files (.ex)
Generated scenic_driver_glfw app
# from Polish: no such file or directory
sh: /home/eiji/scenic_driver_glfw/_build/test/lib/scenic_driver_glfw/priv/test/scenic_driver_glfw: Nie ma takiego pliku ani katalogu

# + of course failed test

@Eiji7
Copy link

Eiji7 commented Jul 21, 2019

Looks like same goes to priv branch:

mkdir -p priv/test
cc -O3 -std=c99 `pkg-config --static --cflags glfw3 glew` -fPIC -o priv/test/scenic_driver_glfw c_src/main.c c_src/comms.c c_src/nanovg/nanovg.c c_src/utils.c c_src/render_script.c c_src/tx.c `pkg-config --static --libs glfw3 glew` -lGL -lm -lrt
Compiling 7 files (.ex)
Generated scenic_driver_glfw app
# again from Polish: no such file or directory
sh: /home/eiji/scenic_driver_glfw/_build/test/lib/scenic_driver_glfw/priv/test/scenic_driver_glfw: Nie ma takiego pliku ani katalogu

# again + of course failed test

@Eiji7
Copy link

Eiji7 commented Jul 21, 2019

@axelson I think that I found problem, but I'm not sure …

In my opinion we have 2 problems:

  1. First time building
  2. Building after only make clean

In 1st case we have compiled scenic_driver_glfw binary and mix does not copies it to _build/${Mix.env}/lib/scenic_driver_glfw/priv/${Mix.env} directory. That's said even _build/${Mix.env}/lib/scenic_driver_glfw/priv/${Mix.env} is not created!

:code.priv_dir(:app_name) points to _build/${Mix.env}/lib/app_name/priv instead of priv.

In 2nd point we have a similar case i.e. binary is created properly without any problem, but not copied by mix:

$ make clean
rm -f -r priv/dev
rm -f -r priv/test
rm -f -r priv/prod
$ mix test
mkdir -p priv/test
cc -O3 -std=c99 `pkg-config --static --cflags glfw3 glew` -fPIC -o priv/test/scenic_driver_glfw c_src/main.c c_src/comms.c c_src/nanovg/nanovg.c c_src/utils.c c_src/render_script.c c_src/tx.c `pkg-config --static --libs glfw3 glew` -lGL -lm -lrt
# again from Polish: no such file or directory
sh: /home/eiji/scenic_driver_glfw/_build/test/lib/scenic_driver_glfw/priv/test/scenic_driver_glfw: Nie ma takiego pliku ani katalogu

When we do only rm -rf _build it would work, because we have not removed priv/app_name and mix properly copies it.

WRONG: Maybe mix generates a list of files to copy before make is called?

EDIT: I found it!
https://github.com/elixir-lang/elixir_make/blob/c8285de328033e3619b7fdb6be37163678bc9757/lib/mix/tasks/compile.make.ex#L126

The real problem is that we already have priv directory and it's not copied by elixir_make! Later it's copied somewhere in mix itself (when build again structure)!

CONFIRMED: Just navigate to deps/elixir_make/lib/mix/tasks/compile.make.ex and temporary comment if and priv? assignment (to not have compile time warning). Once you do it you can run:

# clone repo or clean existing:
$ make clean # removes our binary in priv directory
$ rm -fr _build # removes build directory
# now we have a clean project
$ mix test # now works!

@josevalim
Copy link

Please make sure to bump elixifr_make requirement in mix.exs.

@boydm
Copy link
Collaborator

boydm commented Jul 21, 2019

Thank you @josevalim!
I bumped the deps to {:elixir_make, "~> 0.6"},
And the problem has gone away. Yay!

@ejc123 please add that update to the mix.exs file and lets get this into a release.

Thank you all!

@@ -1,6 +1,8 @@
\MIX = mix
CFLAGS = -O3 -std=c99

PREFIX=$(MIX_COMPILE_PATH)/../priv

Choose a reason for hiding this comment

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

Suggested change
PREFIX=$(MIX_COMPILE_PATH)/../priv
PREFIX=$(MIX_APP_PATH)/priv

@axelson
Copy link

axelson commented Jul 21, 2019

Ah, so the issue was that boyd and I were not running the same version of elixir_make. Would it make sense to commit the mix.lock file to help avoid issues like that in the future?

@boydm
Copy link
Collaborator

boydm commented Jul 21, 2019

@axelson I was just thinking the same thing

@boydm
Copy link
Collaborator

boydm commented Jul 21, 2019

Too tired to get this in tonight. Don't want to make mistakes. Tomorrow...

axelson added a commit to axelson/scenic_driver_glfw that referenced this pull request Jul 21, 2019
Add a mix.lock file with the latest version of depenedencies. Tests run
fine with these for me and they all match `mix.exs`.

Will help bugs arising from contributors using different versions of
dependencies. Similar to what happened in:
ScenicFramework#26
@axelson axelson mentioned this pull request Jul 21, 2019
@axelson
Copy link

axelson commented Jul 21, 2019

Also for future reference, I believe this is the PR that we needed to bring in from elixir_make: elixir-lang/elixir_make#32

Also should this PR remove priv/dev, priv/test, and priv/prod from .gitignore since the code won't be building into it anymore? (since it'll be building into the priv directory in the _build folder)

axelson added a commit to axelson/scenic that referenced this pull request Jul 21, 2019
Includes the latest versions of all direct dependencies

> You will notice that when you add a dependency to your project, Mix
> generates a mix.lock file that guarantees repeatable builds. The lock
> file must be checked in to your version control system, to guarantee
> that everyone who uses the project will use the same dependency versions
> as you.

Source:
https://elixir-lang.org/getting-started/mix-otp/dependencies-and-umbrella-projects.html

Also will help avoid problems like the issues we had testing
ScenicFramework/scenic_driver_glfw#26 in the future.
@Eiji7
Copy link

Eiji7 commented Jul 22, 2019

@axelson Yes, I think so - it's how José see priv working

@boydm boydm merged commit d78949d into ScenicFramework:master Jul 22, 2019
@boydm
Copy link
Collaborator

boydm commented Jul 22, 2019

Hey all. This is now in master. I added Jose's suggestion and make sure the dep on elixir_make is 0.6.

If I could get some confirmations that master works for you on the various systems you use, that would be great. I'll publish an update some time after that.

@Eiji7
Copy link

Eiji7 commented Jul 22, 2019

@boydm I just tested it:

  1. mix test on master works!
  2. mix scenic.new.example example; cd example; mix scenic.run don't work (as expected - i.e. not master)
  3. changing scenic_driver_glfw to master in mix.exs for example from 2nd point and run mix scenic.run works!

My environment:

$ asdf current
elixir         1.9.1-otp-22 (set by …)
erlang         22.0.7   (set by …)
…

Funtoo Linux current x86-64bit on intel64-ivybridge

@axelson
Copy link

axelson commented Jul 22, 2019

@boydm okay, tested scenic.new and my scenic_asteroids repositories with the master branch of scenic_driver_glfw on both Arch Linux and Mac OS 10.14 and they're working great. Although there is a similar issue with scenic_driver_nerves_rpi which I've filed as boydm/scenic_driver_nerves_rpi#10

@boydm
Copy link
Collaborator

boydm commented Jul 28, 2019

This is now published in v0.10.1 to hex

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.

5 participants