Closed Bug 1513450 Opened 9 months ago Closed 9 months ago

Please do not ignore *.rej files

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox66 fixed)

RESOLVED FIXED
Tracking Status
firefox66 --- fixed

People

(Reporter: arai, Assigned: arai)

References

Details

Attachments

(1 file)

Steps to reproduce:
  1. run "hg qpush" with some patch that has conflicting change
  2. run "hg status"

Actual result:
  *.rej files are not listed

Expected result:
  *.rej files are listed as untracked files

This is from bug 1513037, and this breaks my workflow.
I was using "hg status" to list the rejection, to open them one by one and apply each rejected change.

for me, .rej files are most easy way to figure out and solve the conflict (better than internal merge with conflict markers, or external merge tools),
and listing them while fixing rejection helps me a lot to track the progress, and to remove those files after addressing them.

if .rej files are ignored, there's no easy and quick way to list them.
what's the motivation for bug 1513037 ?

to me, ignoring *.orig/*.rej doesn't sound good idea.
if those files appears in any way (even without mq, maybe with raw patch or something),
users are supposed to remove them, with or without addressing the rejection,
but not to leave them as is in the working tree.
Flags: needinfo?(dtownsend)
.orig and .rej files *have* ended up in the tree on multiple occasions because they weren't in .hgignore.

You can use hg status --ignored.
(In reply to Mike Hommey [:glandium] from comment #2)
> .orig and .rej files *have* ended up in the tree on multiple occasions
> because they weren't in .hgignore.

not being listed in .hgignore shouldn't be the reason.
if we want to avoid landing .rej in tree, we should detect those files with commit hook or review bot or something.

leaving them in working tree is already a bad sign,
and ignoring them shouldn't be the solution for such situations
my explanation wasn't clear enough

suppose the case that .rej file is really included in a changeset,
it's possible that the patch author forgot to address that rejection.

if we just ignore .rej file, such mistake will be overlooked.

if we don't ignore, it will be visible in review process and reviewer can point it out.
even if reviewer overlooks, the .rej file will be put into the tree and there's chance to notice it later.

also, with the better solution like detecting them in automations (commit hook or review bot),
patch author will have chance to look into the rejection in early stage.
Out of curiosity, how do .rej files even end up in patches without someone explicitly |hg add|ing them?
The main reasons I added this were:

* the same rule was added to .gitignore (bug 1510363) and consistency seems like a good thing. 
* I've never come across a case where it wasn't obvious that a change had failed to apply (merge and rebase f.e. force you to resolve merge issues), and generally code would fail with merge markers in it.

I don't particularly care either way I guess, it is pretty trivial to work with either case, I just think we should be consistent.
Flags: needinfo?(dtownsend)
(In reply to Emanuel Hoogeveen [:ehoogeveen] from comment #5)
> Out of curiosity, how do .rej files even end up in patches without someone
> explicitly |hg add|ing them?

"hg add ." maybe?
that's also mis-operation I think (patch authors are supposed to remove .rej file before that point)


(In reply to Dave Townsend [:mossop] (he/him) from comment #6)
> The main reasons I added this were:
> 
> * the same rule was added to .gitignore (bug 1510363) and consistency seems
> like a good thing. 
> * I've never come across a case where it wasn't obvious that a change had
> failed to apply (merge and rebase f.e. force you to resolve merge issues),
> and generally code would fail with merge markers in it.
> 
> I don't particularly care either way I guess, it is pretty trivial to work
> with either case, I just think we should be consistent.

Thanks.  then I'd like to suggest removing at least "*.rej" from the list.
I don't have much experience with *.orig files, and I think it's more like backup files (like foo.cpp~),
so I'll leave the decision for *.orig files to others.

I'll create a patch shortly.
Blocks: 1510363
triaging
Assignee: nobody → arai.unmht
Attachment #9030828 - Attachment description: Bug 1513450 - Do not ignore .rej files. r?Mossop → Bug 1513450 - Do not ignore .rej/.orig files. r?nalexander
Pushed by arai_a@mac.com:
https://hg.mozilla.org/integration/autoland/rev/42ac168aa8ec
Do not ignore .rej/.orig files. r=firefox-build-system-reviewers,nalexander
https://hg.mozilla.org/mozilla-central/rev/42ac168aa8ec
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.