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

Lockfiles: figure out how to model the default resolve for multiple user resolves #12742

Closed
Eric-Arellano opened this issue Sep 2, 2021 · 5 comments · Fixed by #13967
Closed
Assignees

Comments

@Eric-Arellano
Copy link
Contributor

Two possible approaches:

  1. Expect the user to name a resolve default, and hardcode resolve field's default to default.
  2. Add an option to let the user set the default named resolve, if any.

In both cases, if the default is not set, then every target will be forced to be explicit.

--

Whatever we do should likely be emulated by "named interpreter constraints".#12652

--

We also need to consider how to model defaults for compatible_resolves with python_library targets: #12714. We should almost certainly let the user choose a default that makes sense for them, rather than forcing them into ['default'] - you may want your default to be >1 element.

That suggests we should let the user decide the default for both the resolve field and compatible_resolves field.

--

In addition to compatible_resolves suggesting the second option is better, I also think there's value in letting the user choose a name that makes sense to their org.

This is probably much more applicable to named interpreter constraints, but I think option 2 gives better modeling when an org wants to change their default to a new value. Rather than having to retcon default, they can simply point to the resolve they want. This is more clear with interpreter constraints: if you start with --default=py2, then you can update it to --default=py3. That's better than changing default to have a different value.

@Eric-Arellano
Copy link
Contributor Author

Interesting! #12784 is looking like Java will use the same "named resolves" mechanism as Python. It would be great for ./pants generate-lockfiles to work the same with both languages, imo. Rather than python-generate-lockfiles vs coursier-generate-lockfiles.

This means that the universe of resolve names would be shared across languages. So, we could not rely on using a resolve name like "default" to know what is the default lockfile. That would be ambiguous across languages. Meaning, we should use a dedicated option for each language to set its default resolve name.

cc @tdyas

@stuhood
Copy link
Member

stuhood commented Nov 14, 2021

#13374 is the relevant ticket for the JVM side of this.

@Eric-Arellano
Copy link
Contributor Author

This means that the universe of resolve names would be shared across languages. So, we could not rely on using a resolve name like "default" to know what is the default lockfile. That would be ambiguous across languages. Meaning, we should use a dedicated option for each language to set its default resolve name.

Or, we can have the defaults be py-resolve and jvm-resolve for example. Option 1 is still a contender and should be considered.

@stuhood
Copy link
Member

stuhood commented Dec 6, 2021

This means that the universe of resolve names would be shared across languages. So, we could not rely on using a resolve name like "default" to know what is the default lockfile. That would be ambiguous across languages. Meaning, we should use a dedicated option for each language to set its default resolve name.

Or, we can have the defaults be py-resolve and jvm-resolve for example. Option 1 is still a contender and should be considered.

Or just py and jvm (which we can lean on subsystem names to reduce collisions for).

Eric-Arellano added a commit that referenced this issue Dec 20, 2021
Part of #12742.

We set a default because it's important to the Pants project that onboarding is as effortless as possible, e.g. via sensible defaults.

An additional benefit is that we can now be confident a resolve is set up, as it is impossible to override an option to be `None` (#11719). We decided that we want to require lockfiles when using Pants, so this is seen as a benefit.

--

This uses the resolve name `jvm-default` because the namespace will be shared with Python, e.g. `py-default`. 

It uses the lockfile path `3rdparty/jvm/default.lock`. We use the file extension `.lock` instead of `.json` to better express the semantics, including that you should not hand-edit the file. Using the `3rdparty/jvm` folder is a sensible default for monorepos, and people can override it to where they want.

[ci skip-rust]
[ci skip-build-wheels]
stuhood pushed a commit to stuhood/pants that referenced this issue Dec 20, 2021
…build#13925)

Part of pantsbuild#12742.

We set a default because it's important to the Pants project that onboarding is as effortless as possible, e.g. via sensible defaults.

An additional benefit is that we can now be confident a resolve is set up, as it is impossible to override an option to be `None` (pantsbuild#11719). We decided that we want to require lockfiles when using Pants, so this is seen as a benefit.

--

This uses the resolve name `jvm-default` because the namespace will be shared with Python, e.g. `py-default`. 

It uses the lockfile path `3rdparty/jvm/default.lock`. We use the file extension `.lock` instead of `.json` to better express the semantics, including that you should not hand-edit the file. Using the `3rdparty/jvm` folder is a sensible default for monorepos, and people can override it to where they want.

[ci skip-rust]
[ci skip-build-wheels]
stuhood added a commit that referenced this issue Dec 20, 2021
…ypick of #13925) (#13930)

Part of #12742.

We set a default because it's important to the Pants project that onboarding is as effortless as possible, e.g. via sensible defaults.

An additional benefit is that we can now be confident a resolve is set up, as it is impossible to override an option to be `None` (#11719). We decided that we want to require lockfiles when using Pants, so this is seen as a benefit.

--

This uses the resolve name `jvm-default` because the namespace will be shared with Python, e.g. `py-default`. 

It uses the lockfile path `3rdparty/jvm/default.lock`. We use the file extension `.lock` instead of `.json` to better express the semantics, including that you should not hand-edit the file. Using the `3rdparty/jvm` folder is a sensible default for monorepos, and people can override it to where they want.

[ci skip-rust]
[ci skip-build-wheels]
@Eric-Arellano Eric-Arellano self-assigned this Dec 21, 2021
@Eric-Arellano
Copy link
Contributor Author

We decided in #13902 to just have --default-resolve and not --default-compatible-resolves. It keeps things a lot simpler.

We decided in #13925 to use the default value of jvm-default: 3rdparty/jvm/default.lock. For Python, we probably want something like py-default: 3rdparty/py/default_lock.txt

Eric-Arellano added a commit that referenced this issue Dec 23, 2021
…olve` (#13967)

Closes #12742.

Similar to #13925, this adds a default of `{"python-default": "3rdparty/python/default_lock.txt"}`. It also lets the user set the default resolve via `--experimental-default-resolve`.

We don't add `--default-compatible-resolves`, as decided in #13902.

## Default file name

The resolve must be unambiguous with JVM, hence using `python-default` instead of `default`.

Everyone agreed in Slack that we should use `python` instead of `py`, e.g. `3rdparty/python` over `3rdparty/py`. The clarity is worth it.

It's unclear to me if we want the file to be called `default_lock.txt`. In JVM, it's called `default.lock`. We could do that here too? I only went with `.txt` to make it a bit more clear to users that they can use these lockfiles just like a normal `requirements.txt` - Python has a strong convention for this.

## Deprecation plan: `--enable-resolves`

We want to require using resolves in the future, as we believe it is important for users to have secure and reproducible builds. But we can't switch it on immediately.

At the same time, to make the feature useful, it's really useful for us to be able to set defaults.

To bridge the gap, `--enable-resolves` feature gates everything. When set, we can act how we will in the future, e.g. assuming that a default resolve must always be set. When unset, we use the status quo. In Pants 2.10, we will deprecate not setting this option so that we can make the default in Pants 2.11 be to use resolves.

[ci skip-rust]
[ci skip-build-wheels]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants