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

Compilation error when export deps_path is set #113

Closed
warrenseine opened this issue Feb 17, 2022 · 13 comments
Closed

Compilation error when export deps_path is set #113

warrenseine opened this issue Feb 17, 2022 · 13 comments
Labels
bug Something isn't working

Comments

@warrenseine
Copy link

When deps_path is set to a directory outside of the project workspace, build and hook installation fail. I suppose that the library is installed there and the git rev-parse --git-path "" command is executed in that directory, which is not a Git repository indeed. See logs:

==> git_hooks
Compiling 16 files (.ex)
↗ Installing git hooks...
fatal: not a git repository (or any parent up to mount point /home/app)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).

== Compilation error in file lib/git_hooks.ex ==
** (MatchError) no match of right hand side value: {"", 128}
    lib/git/path.ex:50: GitHooks.Git.Path.resolve_git_path_based_on_git_version/1
    lib/git/path.ex:15: GitHooks.Git.Path.resolve_app_path/0
    lib/mix/tasks/git_hooks/install.ex:48: Mix.Tasks.GitHooks.Install.install/1
    lib/git_hooks.ex:7: (module)
could not compile dependency :git_hooks, "mix compile" failed. You can recompile this dependency with "mix deps.compile git_hooks", update it with "mix deps.update git_hooks" or clean it with "mix deps.clean git_hooks"
@qgadrian qgadrian added the bug Something isn't working label Feb 18, 2022
@qgadrian
Copy link
Owner

qgadrian commented Feb 18, 2022

When deps_path is set to a directory outside of the project workspace, build and hook installation fail. I suppose that the library is installed there and the git rev-parse --git-path "" command is executed in that directory, which is not a Git repository indeed. See logs:

==> git_hooks
Compiling 16 files (.ex)
↗ Installing git hooks...
fatal: not a git repository (or any parent up to mount point /home/app)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).

== Compilation error in file lib/git_hooks.ex ==
** (MatchError) no match of right hand side value: {"", 128}
    lib/git/path.ex:50: GitHooks.Git.Path.resolve_git_path_based_on_git_version/1
    lib/git/path.ex:15: GitHooks.Git.Path.resolve_app_path/0
    lib/mix/tasks/git_hooks/install.ex:48: Mix.Tasks.GitHooks.Install.install/1
    lib/git_hooks.ex:7: (module)
could not compile dependency :git_hooks, "mix compile" failed. You can recompile this dependency with "mix deps.compile git_hooks", update it with "mix deps.update git_hooks" or clean it with "mix deps.clean git_hooks"

That is correct, in that case, I can handle the error and default to .git/hooks (ef28111). Will that work on your case or do we need to be more flexible in this case?

@warrenseine
Copy link
Author

This would work perfectly! Thank you!

@qgadrian
Copy link
Owner

Fixed on latest release v0.7.1, published on hex.pm as well

@warrenseine
Copy link
Author

warrenseine commented Feb 18, 2022

For some reason, it is still failing (differently) with 0.7.1:

==> git_hooks
Compiling 16 files (.ex)
↗ Installing git hooks...
fatal: not a git repository (or any parent up to mount point /home/app)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
fatal: not a git repository (or any parent up to mount point /home/app)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
fatal: not a git repository (or any parent up to mount point /home/app)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
fatal: not a git repository (or any parent up to mount point /home/app)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
fatal: not a git repository (or any parent up to mount point /home/app)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
fatal: not a git repository (or any parent up to mount point /home/app)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
fatal: not a git repository (or any parent up to mount point /home/app)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
Generated git_hooks app
app@3c45cfbe7dcc:/workspace$ cat .git/hooks/pre-commit
cat: .git/hooks/pre-commit: No such file or directory

It does not crash, but still does not generate the pre-commit file in my workspace. It actually generates it under the deps_path, in my case ~/elixir-artifacts/deps:

app@3c45cfbe7dcc:/workspace$ cat ~/elixir-artifacts/deps/git_hooks/.git/hooks/pre-commit 
#!/bin/sh

[ "/home/app/elixir-artifacts/deps/git_hooks" != "" ] && cd /home/app/elixir-artifacts/deps/git_hooks

mix git_hooks.run pre_commit "$@"
[ $? -ne 0 ] && exit 1
exit 0

@warrenseine
Copy link
Author

warrenseine commented Feb 18, 2022

Interestingly, I noted that System.get_env("PWD") returns the project path (what we want), while File.cwd!() returns the dependency path.

Meaning that we could wrap the git call with:

defp git(args) do
  File.cd!(System.get_env("PWD", File.cwd!()), fn () ->
    "git"
    |> System.cmd(args)
  end)
end

But that's not enough. The whole install() function has to be wrapped as well, otherwise the File.write() still writes at the wrong location.

@warrenseine
Copy link
Author

warrenseine commented Feb 18, 2022

Yes, just wrapping everything works for me locally if I just do:

File.cd!(System.get_env("PWD", File.cwd!()), fn () ->
  install(opts)
end)

I don't have all the context for this project, so I prefer to let you patch.

@qgadrian qgadrian reopened this Feb 20, 2022
@qgadrian
Copy link
Owner

thanks for the information @warrenseine, I will reopen the issue and update you back once I have more information/fix

@qgadrian
Copy link
Owner

@warrenseine I understand the following, please correct me if I'm wrong:

  • You have a project under a certain path, for example, ~/workspace/the-project
  • You have a deps path external to the project above, for example, ~/workspace/elixir-artifacts/deps/the-project

I think it's best to only support the "regular project architecture" (mono repo project, relative deps folder, etc...) but allow this project to be additionally configured for each different scenario. That will help me to keep the code simple and easier to maintain and contribute ☺️

Can you please give it a try to #120 and tell me it's enough for your case? Thanks mate!

@warrenseine
Copy link
Author

Yes, you understood correctly.

My use case is simple: I'm using Docker Compose, which seems to be a supported use case! My workspace is on the host, and shared with the guest. To speed up the build, I keep the artifacts/dependencies inside the Docker container, so they don't get synchronized with the workspace (as they would if I used the default relative deps path).

I totally get your point, and I think your solution is better. I will try the configuration setting!

@warrenseine
Copy link
Author

If I configure my project like that:

if Mix.env() == :dev do
  config :git_hooks,
    hooks: [
      pre_commit: [
        tasks: [
          {:cmd, "mix format --check-formatted"}
        ]
      ]
    ],
    project_path: File.cwd!()
end

I can see that the relative path is now correctly pointing to my workspace, rather than the dependency path. However, when git is executed, we're still in the dependency path, so it still fails with fatal: not a git repository. I think we still need to cd into the project path before calling git.

I made a reproduction case in this repo if you want to test it. Just checkout and run rm -rf _build && mix deps.get && mix deps.compile && cat .git/hooks/pre-commit (will fail with "no such file or directory").

@qgadrian
Copy link
Owner

qgadrian commented Apr 1, 2022

If I configure my project like that:

if Mix.env() == :dev do
  config :git_hooks,
    hooks: [
      pre_commit: [
        tasks: [
          {:cmd, "mix format --check-formatted"}
        ]
      ]
    ],
    project_path: File.cwd!()
end

I can see that the relative path is now correctly pointing to my workspace, rather than the dependency path. However, when git is executed, we're still in the dependency path, so it still fails with fatal: not a git repository. I think we still need to cd into the project path before calling git.

I made a reproduction case in this repo if you want to test it. Just checkout and run rm -rf _build && mix deps.get && mix deps.compile && cat .git/hooks/pre-commit (will fail with "no such file or directory").

Thanks for the example project, that was super helpful.

I updated #120 with a fix, the README has an example of the configuration to make this particular case.

I couldn't find any better way to provide the project path to the library, if you do please let me know and I will change it 🙌

@warrenseine
Copy link
Author

I tested locally and can confirm it's now working for my use case! Thank you very much!

@qgadrian
Copy link
Owner

qgadrian commented Apr 6, 2022

I tested locally and can confirm it's now working for my use case! Thank you very much!

awesome!

Fixed in the latest release https://github.com/qgadrian/elixir_git_hooks/releases/tag/v0.7.3

@qgadrian qgadrian closed this as completed Apr 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants