Skip to content

Conversation

vrigal
Copy link
Collaborator

@vrigal vrigal commented Jul 18, 2025

Ref. #2887

The hglib client only supports the similarity parameter (and not the config option): https://repo.mercurial-scm.org/python-hglib/file/tip/hglib/client.py#l959. Therefore, I could not implement the fall back command in case the command fails. It should be possible with a raw call to hg, but I suppose we want to avoid that.

I did not added the --no-commit option as Bastien told me it would work only on try.

@vrigal vrigal requested a review from marco-c July 18, 2025 13:26
@marco-c marco-c linked an issue Jul 18, 2025 that may be closed by this pull request
La0
La0 previously approved these changes Jul 18, 2025
@La0 La0 requested a review from Archaeopteryx July 18, 2025 13:38
@marco-c
Copy link
Collaborator

marco-c commented Jul 18, 2025

We could use hg_cmd to implement the fallback without hglib

@marco-c
Copy link
Collaborator

marco-c commented Jul 28, 2025

@cgsheeh is this still the way Lando does it?

Lando does an hg import --no-commit -s 95 and if that fails, it then runs a hg import --no-commit --config ui.patch=patch -s 95, which falls back to using the patch command

@cgsheeh
Copy link
Member

cgsheeh commented Jul 28, 2025

@cgsheeh is this still the way Lando does it?

Lando does an hg import --no-commit -s 95 and if that fails, it then runs a hg import --no-commit --config ui.patch=patch -s 95, which falls back to using the patch command

Yes, this still is how Lando applies patches to Mercurial repos.

@vrigal
Copy link
Collaborator Author

vrigal commented Aug 21, 2025

@marco-c I did an implementation of the fallback case directly with hg_run.
I'm trying to run it locally, but it's difficult to test.

@vrigal
Copy link
Collaborator Author

vrigal commented Aug 21, 2025

I had trouble to fix one test so I marked it as skipped (allow failure).

Also I noticed the above error in another CI job:

apt install -qq -y postgresql-15
Error: Unable to locate package postgresql-15

It may be caused by the new Debian release that does not support postgresql-15. We can either pin the CI image, or bump to postgres 17.
Ref.: https://github.com/mozilla/code-review/pull/2886/checks?check_run_id=48584125630

@La0
Copy link
Collaborator

La0 commented Aug 28, 2025

You should be able to rebase on top of #2941

@vrigal
Copy link
Collaborator Author

vrigal commented Sep 4, 2025

@marco-c could you take a look at the fallback implementation please?
The code is quite simple, but I could not test it with a local patch that would trigger the hglib.error.CommandError (I'm also not 100% certain that this is the correct exception to catch the mentioned failure).

@marco-c
Copy link
Collaborator

marco-c commented Sep 4, 2025

@cgsheeh could you have a look?

@marco-c marco-c requested a review from cgsheeh September 4, 2025 15:42
Copy link
Collaborator

@La0 La0 left a comment

Choose a reason for hiding this comment

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

LGTM, especially with a unit test trigerring both imports

@La0 La0 requested a review from cgsheeh September 5, 2025 11:56
Copy link
Member

@cgsheeh cgsheeh left a comment

Choose a reason for hiding this comment

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

LGTM. One thing to note, if hg import fails and we then run with patch and fail again, the raised exception will be from patch instead of hg import.

@marco-c marco-c merged commit feaaaec into mozilla:master Sep 5, 2025
10 checks passed
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.

Apply patches from Phabricator in a smarter way
4 participants