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

Can't push changes to a repo that already has a change with an empty log message #2633

Closed
durin42 opened this issue Nov 25, 2023 · 12 comments · Fixed by #3653
Closed

Can't push changes to a repo that already has a change with an empty log message #2633

durin42 opened this issue Nov 25, 2023 · 12 comments · Fixed by #3653
Labels
good first issue Good for newcomers

Comments

@durin42
Copy link
Collaborator

durin42 commented Nov 25, 2023

Description

I wanted to push a trivial cleanup change to a friend's project, but this commit already deep in the history has no log message, and prevents me from pushing to my fork entirely.

Steps to Reproduce the Problem

  1. Make a change to an existing history that has a buried empty log message
  2. Try to push

Expected Behavior

The push isn't blocked because the outgoing set doesn't contain any empty log messages.

Actual Behavior

Creating branch push-wsksktrvyzky for revision wsks
Error: Won't push commit dcc574ca60a9 since it has no description

Specifications

  • Platform: macOS 14.1.1, aarch64
  • Version: 4f2503c
@durin42
Copy link
Collaborator Author

durin42 commented Nov 25, 2023

(you can work around this by doing fetch on the remote before push, but this took me thinking like a git implementor to figure out, so probably is a rough edge that should be fixed)

@martinvonz
Copy link
Member

Oh, fun.

We could make it depend on whether the commit was created by jj or just imported from git, but we don't keep track of that right now. Actually, we can kind of tell from the change id; if the change id is a bit-reversed commit id, then it's imported. But relying on that would be a bit of a hack.

Another option is to simply print a hint about first running jj git fetch. However, we'd then still prevent pushing to a remote that doesn't have the commit.

So maybe the best option is to add a --allow-empty-description flag (we have talked about having a --allow-conflicts flag too, btw).

@martinvonz martinvonz added the good first issue Good for newcomers label Nov 25, 2023
@ilyagr
Copy link
Collaborator

ilyagr commented Nov 25, 2023 via email

@martinvonz
Copy link
Member

We could also silently allow pushing empty descriptions for immutable commits.

We basically already do. We only look for empty descriptions in [email protected] (or whatever you're pushing). The problem above happens when you push without ever having fetched from the remote (or at least not having fetched the problematic commit from it).

@durin42
Copy link
Collaborator Author

durin42 commented Nov 25, 2023

Suggesting fetch seems like a worthy thing. Is there a reason to not auto-fetch before doing the push?

@martinvonz
Copy link
Member

Is there a reason to not auto-fetch before doing the push?

Good question. Fetching becomes slow over time due to all the refs/jj/keep/* refs we create (#293), so maybe that's the main reason not to. Once we've fixed that, we can probably auto-fetch before push.

@durin42
Copy link
Collaborator Author

durin42 commented Nov 25, 2023 via email

@mgattozzi
Copy link
Contributor

So maybe the best option is to add a --allow-empty-description flag (we have talked about having a --allow-conflicts flag too, btw).

I ran into this today as well and had to fall back to using git so I could do the push. I can't control what others do with their history. It's a great sanity check and having the option to opt out for edge cases like this would be great.

@martinvonz
Copy link
Member

Have time to send a PR, @mgattozzi?

@mgattozzi
Copy link
Contributor

Yeah I'd love to @martinvonz. Where would be the best place to make those changes so I can add this flag?

@mgattozzi
Copy link
Contributor

Awesome thanks for the pointers. I'll be able to get to it next week 😁

mgattozzi added a commit to mgattozzi/jj that referenced this issue May 9, 2024
This commit adds an optional flag to be able to push commits with an
empty description to a remote git repo. While the default behavior is
ideal we might need to interact with a repo that has an empty commit
description in it. I ran into this issue a few weeks ago pushing commits
from an open source repo to an empty repo and had to go back to using
git for that push as I would not want to rewrite the history which was
many many years long just for that.

This flag allows users an escape hatch for pushing empty descriptions
for commits and they're sure that they want that behavior.

This commit adds the flag to the `git push` command and updates the docs
for the command. It also adds a new test to make sure that the flag
works as intended alongside the old test which makes sure to reject an
empty message for a commit.

Closes jj-vcs#2633
mgattozzi added a commit to mgattozzi/jj that referenced this issue May 9, 2024
This commit adds an optional flag to be able to push commits with an
empty description to a remote git repo. While the default behavior is
ideal we might need to interact with a repo that has an empty commit
description in it. I ran into this issue a few weeks ago pushing commits
from an open source repo to an empty repo and had to go back to using
git for that push as I would not want to rewrite the history which was
many many years long just for that.

This flag allows users an escape hatch for pushing empty descriptions
for commits and they're sure that they want that behavior.

This commit adds the flag to the `git push` command and updates the docs
for the command. It also adds a new test to make sure that the flag
works as intended alongside the old test which makes sure to reject an
empty message for a commit.

Closes jj-vcs#2633
mgattozzi added a commit to mgattozzi/jj that referenced this issue May 10, 2024
This commit adds an optional flag to be able to push commits with an
empty description to a remote git repo. While the default behavior is
ideal we might need to interact with a repo that has an empty commit
description in it. I ran into this issue a few weeks ago pushing commits
from an open source repo to an empty repo and had to go back to using
git for that push as I would not want to rewrite the history which was
many many years long just for that.

This flag allows users an escape hatch for pushing empty descriptions
for commits and they're sure that they want that behavior.

This commit adds the flag to the `git push` command and updates the docs
for the command. It also adds a new test to make sure that the flag
works as intended alongside the old test which makes sure to reject an
empty message for a commit.

Closes jj-vcs#2633
mgattozzi added a commit to mgattozzi/jj that referenced this issue May 16, 2024
This commit adds an optional flag to be able to push commits with an
empty description to a remote git repo. While the default behavior is
ideal we might need to interact with a repo that has an empty commit
description in it. I ran into this issue a few weeks ago pushing commits
from an open source repo to an empty repo and had to go back to using
git for that push as I would not want to rewrite the history which was
many many years long just for that.

This flag allows users an escape hatch for pushing empty descriptions
for commits and they're sure that they want that behavior.

This commit adds the flag to the `git push` command and updates the docs
for the command. It also updates the original test to make sure that the
flag works as intended to reject the commit when not set and to allow
the commit when the flag is set.

Closes jj-vcs#2633
mgattozzi added a commit to mgattozzi/jj that referenced this issue May 16, 2024
This commit adds an optional flag to be able to push commits with an
empty description to a remote git repo. While the default behavior is
ideal we might need to interact with a repo that has an empty commit
description in it. I ran into this issue a few weeks ago pushing commits
from an open source repo to an empty repo and had to go back to using
git for that push as I would not want to rewrite the history which was
many many years long just for that.

This flag allows users an escape hatch for pushing empty descriptions
for commits and they're sure that they want that behavior.

This commit adds the flag to the `git push` command and updates the docs
for the command. It also updates the original test to make sure that the
flag works as intended to reject the commit when not set and to allow
the commit when the flag is set.

Closes jj-vcs#2633
mgattozzi added a commit to mgattozzi/jj that referenced this issue Jun 3, 2024
This commit adds an optional flag to be able to push commits with an
empty description to a remote git repo. While the default behavior is
ideal we might need to interact with a repo that has an empty commit
description in it. I ran into this issue a few weeks ago pushing commits
from an open source repo to an empty repo and had to go back to using
git for that push as I would not want to rewrite the history which was
many many years long just for that.

This flag allows users an escape hatch for pushing empty descriptions
for commits and they're sure that they want that behavior.

This commit adds the flag to the `git push` command and updates the docs
for the command. It also updates the original test to make sure that the
flag works as intended to reject the commit when not set and to allow
the commit when the flag is set.

Closes jj-vcs#2633
mgattozzi added a commit to mgattozzi/jj that referenced this issue Jun 5, 2024
This commit adds an optional flag to be able to push commits with an
empty description to a remote git repo. While the default behavior is
ideal we might need to interact with a repo that has an empty commit
description in it. I ran into this issue a few weeks ago pushing commits
from an open source repo to an empty repo and had to go back to using
git for that push as I would not want to rewrite the history which was
many many years long just for that.

This flag allows users an escape hatch for pushing empty descriptions
for commits and they're sure that they want that behavior.

This commit adds the flag to the `git push` command and updates the docs
for the command. It also updates the original test to make sure that the
flag works as intended to reject the commit when not set and to allow
the commit when the flag is set.

Closes jj-vcs#2633
emesterhazy pushed a commit that referenced this issue Jun 5, 2024
This commit adds an optional flag to be able to push commits with an
empty description to a remote git repo. While the default behavior is
ideal we might need to interact with a repo that has an empty commit
description in it. I ran into this issue a few weeks ago pushing commits
from an open source repo to an empty repo and had to go back to using
git for that push as I would not want to rewrite the history which was
many many years long just for that.

This flag allows users an escape hatch for pushing empty descriptions
for commits and they're sure that they want that behavior.

This commit adds the flag to the `git push` command and updates the docs
for the command. It also updates the original test to make sure that the
flag works as intended to reject the commit when not set and to allow
the commit when the flag is set.

Closes #2633
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants