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

Committing inside a git submodule fails #33

Closed
3 tasks done
cjlarose opened this issue Jan 10, 2019 · 7 comments
Closed
3 tasks done

Committing inside a git submodule fails #33

cjlarose opened this issue Jan 10, 2019 · 7 comments

Comments

@cjlarose
Copy link
Contributor

Prerequisites

Description

Steps to Reproduce

Checkout git-mob at revision cd74de0. npm install; npm link.

cat <<-EOF > ~/.git-coauthors
{
  "coauthors": {
    "ad": {
      "name": "Amy Doe",
      "email": "amy@findmypast.com"
    },
    "bd": {
      "name": "Bob Doe",
      "email": "bob@findmypast.com"
    }
  }
}
EOF
cd ~/dev
mkdir parent-repo
git init
git submodule add git@github.com:findmypast-oss/git-mob.git child-repo
cd child-repo
git mob ad bd
git commit

Expected behavior: git opens up my $EDITOR with the Co-Authored-By trailers already in my commit message

Actual behavior:

fatal: could not read '.git/.gitmessage': Not a directory

Reproduces how often: Every time

Versions

  • operating system and version: macOS Mojave 10.14.2
  • git-mob version (git-mob --version): 0.4.0
  • git version (git --version): git version 2.20.1

Additional Information

git mob ad bd correctly updates the git config for the submodule at ~/dev/parent-repo/.git/modules/child-repo/config to include

[commit]
        template = .git/.gitmessage
[git-mob]
        co-author = Amy Doe <amy@findmypast.com>
        co-author = Bob Doe <bob@findmypast.com>

Here, .git/.gitmessage is a path I think is meant to be relative to the child
repository's root (~/dev/parent-repo/child-repo), and git does appear to be trying to read the file at ~/dev/parent-repo/child-repo/.git/.gitmessage.

However, because child-repo is a submodule, child-repo/.git is not a directory. The .gitmessage file is actually at ~/dev/parent-repo/.git/modules/child-repo/.gitmessage.

@cjlarose
Copy link
Contributor Author

cjlarose commented Jan 10, 2019

I wanted to open up a PR to solve this issue, but I couldn't think of a straightforward and uncontroversial way to make this work.

If we make it so that git config commit.template references an absolute path, that path will break whenever the repo gets moved around or even if you rename your submodule. There doesn't seem to be a way to make it so that git config commit.template is interpreted relative to the .git directory (for example $GIT_DIR doesn't expand when used as part of this config value).

Using a path relative to the .git directory to store the .gitmessage file is certainly desirable because it lets us use project-specific authors; changing git config commit.template to be a global path like ~/.git-mob-message-template would break that use case.

The best solution I can think of right now is to set git config commit.template to something like ~/.git-mob-templates/ABD123DEF using a randomly-generated filename for each project. This approach is resilient to submodule renames and also continues to support project-specific templates, but that seemed like a pretty major change. If there's something simpler we can do, that'd be awesome.

Edit: Ok so I thought of a much simpler solution. I don't know why, but earlier I was thinking that the value stored for commit.template was static. In reality, it can be updated every time we git mob or git solo. I think an elegant solution to this problem is to do something like

git config set commit.template $(git rev-parse --git-path '.gitmessage')

every time we git mob or git solo. That way the path stored in commit.template is always correct.

I'll see if I can work on a PR.

cjlarose added a commit to cjlarose/git-mob that referenced this issue Jan 10, 2019
This is done in order to support commiting from git submodules. Fixes rkotze#33
@cjlarose cjlarose changed the title Comminting inside a git submodule fails Committing inside a git submodule fails Jan 10, 2019
cjlarose added a commit to cjlarose/git-mob that referenced this issue Jan 10, 2019
This is done in order to support committing from git submodules. If the
repository is a submodule, then `git config commit.template` will be an
absolute path to the `.gitmessage` file. If the repository is not a
submodule, it will be a path relative to the repository root (consistent
with current behavior).

Fixes rkotze#33
cjlarose added a commit to cjlarose/git-mob that referenced this issue Jan 10, 2019
This is done in order to support committing from git submodules.

Fixes rkotze#33
@rkotze
Copy link
Owner

rkotze commented Jan 11, 2019

Thanks for the detailed issue. Interesting bug. Look forward to the PR.

@rkotze
Copy link
Owner

rkotze commented Jan 17, 2019

I've taken (@cjlarose ) commit to contribute to solving this issue and merged it in. Also made a tweak to it.

Now released in v0.5.1

@rkotze rkotze closed this as completed Jan 17, 2019
@cjlarose
Copy link
Contributor Author

Thanks for merging that change! I meant to open a PR earlier, but wanted to write some tests. Didn't get around to it, but happy to see the changes landed in the most recent version!

@rkotze
Copy link
Owner

rkotze commented Jan 18, 2019

I imagine there would be a lot of setup it which might make it slow. But It will be good to have a test to cover it. 👍

@cjlarose
Copy link
Contributor Author

Would you be opposed to having some "integration level" tests that actually created new git repositories via git init? I found that the project didn't have any so the tests were essentially bypassing a bunch of code that's actually used in practice.

@rkotze
Copy link
Owner

rkotze commented Jan 18, 2019

I agree it should be an integration test as it will be the most valuable test for this case.

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

No branches or pull requests

2 participants