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

v2-only mode should not acknowledge v1 goals #8518

Merged
merged 6 commits into from Nov 7, 2019

Conversation

benjyw
Copy link
Sponsor Contributor

@benjyw benjyw commented Oct 21, 2019

Previously all v1 goals were included when validating
the requested goals, even with --no-v1.

This change:

  1. Only accepts requests for v2 goals if --no-v1 --v2 is set.
  2. Only lists v2 goals for ./pants list, if --no-v1 --v2 is set.

This leads to a more isolated "v2-only" experience.

Previously all v1 goals were included when validating
the requested goals, even with --no-v1.

This change:

1. Only accepts requests for v2 goals if --no-v1 --v2 is set.
2. Only lists v2 goals for `./pants list`, if --no-v1 --v2 is set.

This leads to a more isolated "v2-only" experience.
@benjyw benjyw changed the title Improve v2 goal-related help messages. v2-only mode should not acknowledge v1 goals Oct 21, 2019
@stuhood
Copy link
Sponsor Member

stuhood commented Oct 22, 2019

@benjyw : I haven't given much thought to how we (Twitter, but any other company with a mixed repository) will migrate across the "python is v2, everything else is v1" boundary... it would be good to do that before continuing to tweak these options, as they were implemented a while back when that question was much more hypothetical than it is now.

@benjyw
Copy link
Sponsor Contributor Author

benjyw commented Oct 22, 2019

@benjyw : I haven't given much thought to how we (Twitter, but any other company with a mixed repository) will migrate across the "python is v2, everything else is v1" boundary... it would be good to do that before continuing to tweak these options, as they were implemented a while back when that question was much more hypothetical than it is now.

Well, this should only affect things when turning v1 off entirely (--no-v1), which I guess is not the case if you have a mixed repo?

But I think that --no-v1 really should turn off the v1 engine as much as possible.

@stuhood
Copy link
Sponsor Member

stuhood commented Oct 22, 2019

We have all day meetings today, so had only a few minutes to think about this, but here are some notes:

Imagining a period where JVM and JS are v1, but Python has been ported to v2:

  • --v1 --v2 : will run everything
  • --no-v1 --v2: will run only Python tests
  • --v1 --no-v2: will run everything but Python tests

In the current state, when Python has been "partially ported": how does v2 "become the default" for Python?

  • Cannot globally set --v1 --v2, because would cause tests to double-run in v1 and v2 test
  • Would need to ask beta python users Python users to pass --no-v1 --v2 to all commands...?
    • Cannot make this the default for the repo, because would break JVM/JS users

So... it feels like these flags need to somehow become language or backend-aware, rather than being global.

@benjyw
Copy link
Sponsor Contributor Author

benjyw commented Oct 25, 2019 via email

@benjyw
Copy link
Sponsor Contributor Author

benjyw commented Oct 27, 2019

On Tue, Oct 22, 2019 at 1:02 PM Stu Hood @.***> wrote: - Cannot globally set --v1 --v2, because would cause tests to double-run in v1 and v2 test

Why would we keep the v1 python test task around though?

This change makes the --v1 --v2 flags live up to their descriptions:

    # Toggles v1/v2 `Task` vs `@rule` pipelines on/off.
    register('--v1', advanced=True, type=bool, default=True,
             help='Enables execution of v1 Tasks.')
    register('--v2', advanced=True, type=bool, default=False,
             help='Enables execution of v2 @console_rules.')

Copy link
Sponsor Member

@stuhood stuhood left a comment

Choose a reason for hiding this comment

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

Why would we keep the v1 python test task around though?

We'll need to until it's feature complete, and/or until we've deprecated the relevant flags on test.pytest.

This change makes the --v1 --v2 flags live up to their descriptions:

From that perspective, I'm fine with it. I'm just pointing out that this will not be sufficient to accomplish a migration from v1 to v2, in general: the flags will need further changes.

It doesn't seem to make things any harder though, so I won't block it.

@stuhood
Copy link
Sponsor Member

stuhood commented Oct 28, 2019

Opened #8550 about that followup.

@benjyw benjyw merged commit f4cf055 into pantsbuild:master Nov 7, 2019
@benjyw benjyw deleted the v2_goal_help branch November 7, 2019 21:17
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.

None yet

4 participants