mach try again doesn't work correctly with multiple checkouts

RESOLVED FIXED in Firefox -esr60

Status

defect
P3
normal
RESOLVED FIXED
9 months ago
5 months ago

People

(Reporter: gabriel-v, Assigned: gabriel-v)

Tracking

Version 3
mozilla63
Points:
---

Firefox Tracking Flags

(firefox-esr60 fixed, firefox63 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

9 months ago
The history file for 'mach try again' is under the state dir, '~/.mozconfig', which means it's shared between source clones.

Steps to reproduce:

1. repo A: mach try commandA
2. repo B: mach try commandB
3. repo A: mach try again

Outcome:  step 3 runs commandB.
Expected: step 3 runs commandA.

I propose to move the history file to the topsrcdir.
Comment hidden (mozreview-request)
(Assignee)

Updated

9 months ago
Assignee: nobody → tvijiala
(Assignee)

Updated

9 months ago
Attachment #8996977 - Flags: review?(gbrown)
(Assignee)

Updated

9 months ago
See Also: → 1474869
(Assignee)

Updated

9 months ago
Attachment #8996977 - Flags: review?(gbrown)
Comment hidden (mozreview-request)
(Assignee)

Updated

9 months ago
Attachment #8996977 - Attachment is obsolete: true
(Assignee)

Updated

9 months ago
Status: NEW → ASSIGNED

Comment 5

9 months ago
mozreview-review
Comment on attachment 8997829 [details]
Bug 1480187 - Fix mach try again when using multiple source checkouts.

https://reviewboard.mozilla.org/r/261542/#review269088

::: tools/tryselect/push.py:49
(Diff revision 1)
>  
>  here = os.path.abspath(os.path.dirname(__file__))
>  build = MozbuildObject.from_environment(cwd=here)
>  vcs = get_repository_object(build.topsrcdir)
> -history_path = os.path.join(get_state_dir()[0], 'history', 'try_task_configs.json')
> +topsrcdir_hash = hashlib.sha256(os.path.abspath(build.topsrcdir)).hexdigest()
> +history_path = os.path.join(get_state_dir()[0], 'history', '{}.json'.format(topsrcdir_hash))

Please make the history_path `history/<hash>/try_task_configs.json`. Will make it a bit easier to store other stuff.

::: tools/tryselect/push.py:60
(Diff revision 1)
>          json.dump(try_task_config, fh, indent=2, separators=(',', ':'))
>          fh.write('\n')
>      return config_path
>  
>  
>  def write_task_config_history(msg, try_task_config):

Could you add an if statement that detects if the old `history_path` exists and if so moves it to the new location?

That way people don't lose their history during the transition.
Attachment #8997829 - Flags: review?(ahal) → review-
Comment hidden (mozreview-request)
(Assignee)

Comment 8

8 months ago
mozreview-review-reply
Comment on attachment 8997829 [details]
Bug 1480187 - Fix mach try again when using multiple source checkouts.

https://reviewboard.mozilla.org/r/261542/#review269088

> Could you add an if statement that detects if the old `history_path` exists and if so moves it to the new location?
> 
> That way people don't lose their history during the transition.

I had to add this to selectors/again.py, so the move happens before the history is read or purged.

Comment 9

8 months ago
mozreview-review
Comment on attachment 8997829 [details]
Bug 1480187 - Fix mach try again when using multiple source checkouts.

https://reviewboard.mozilla.org/r/261542/#review269440

Thanks, lgtm!
Attachment #8997829 - Flags: review?(ahal) → review+
(Assignee)

Updated

8 months ago
Keywords: checkin-needed

Comment 10

8 months ago
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/autoland/rev/5312e71473c4
Fix mach try again when using multiple source checkouts. r=ahal

Comment 11

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5312e71473c4
Status: ASSIGNED → RESOLVED
Last Resolved: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.