1480187 - mach try again doesn't work correctly with multiple checkouts

Status: RESOLVED FIXED

(Testing :: General, defect, P3)

Description

[Gabriel Vîjială [:gabriel-v]]

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 1

[Gabriel Vîjială [:gabriel-v]]

Attachment: Bug 1480187 - Move mach try again history file to topsrcdir.

Review commit: https://reviewboard.mozilla.org/r/260954/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/260954/

Comment 2

[Gabriel Vîjială [:gabriel-v]]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c9df41679ef83651168e9374f484fbd9623508a2

Comment 3

[Gabriel Vîjială [:gabriel-v]]

Attachment: Bug 1480187 - Fix mach try again when using multiple source checkouts.

Review commit: https://reviewboard.mozilla.org/r/261542/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/261542/

Comment 4

[Gabriel Vîjială [:gabriel-v]]

updated try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6ec5b91df144aabf29f8e59a536cf0386951c86c

Comment 5

[Andrew Halberstadt [:ahal]]

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.

Comment 6

[Gabriel Vîjială [:gabriel-v]]

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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/261542/diff/1-2/

Comment 7

[Gabriel Vîjială [:gabriel-v]]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6f0f5ca451e68ed5f3b2c2c5856969ce15013d07

Comment 8

[Gabriel Vîjială [:gabriel-v]]

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

[Andrew Halberstadt [:ahal]]

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!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5312e71473c4
Fix mach try again when using multiple source checkouts. r=ahal

Comment 11

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/5312e71473c4

Comment 12

[Tom Prince [:tomprince]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/bb24d02d370b