search

JuliaLang / Pkg.jl

Pkg - Package manager for the Julia programming language
https://pkgdocs.julialang.org
Other
613 stars 251 forks source link

rename [extras] to [test] and get rid of [targets] #3784

Closed oxinabox closed 5 months ago

oxinabox commented 5 months ago

@nickrobinson251 asked me to write this up

Right now in order to use a test time dependency you need to

  1. add it's name and UUID to [extras]
  2. add its name to the test array under [targets]

This duplication of information is annoying. Not very annoying but a little annoying. [extras] right now is only used for listing names and UUIDs of test time dependencies. Nothing else. It seems like we just don't need the extra listing of all their names again in targets.test.

Concerns:

One thing that is different between the lists (at least in theory) is that on some version of julia you can list things in targets.test that are not in [extras] if they are in [weakdeps]. At least according to manual, but it doesn't say what versions. Idk if anyone is relying on this.

A counter argument to this is that folk should stop using the [extras]/[targets] section and use test/Project.toml. However myself and I know many others actually prefer everything in one file -- makes it harder to mistakenly do funny things with [compat].

Deprecation path

Proposal 1.

We would continue to allow [extras] but it would just be an alias for [test]. And we would continue to allow [targets] but it would just be ignored -- even if you used it with [extras] we would still treat the list under [extras] as the list of things to install for running Pkg.test. This would technically i think result in some code which lists things in [extras] but doesn't list them in targets.test to download extra files. But having such extra entries in [extras] is probably only occurring by mistake, and it will have no observable effect on the actual running of the code.

Proposal 2.

We would continue to allow [extras] but it would just be an alias for [test]. If targets.test is present then we would use it as present. Otherwise we would just automatically populate it with keys(test). This has the advantage that we would allow people to continue not listing [weakdeps] in [extras].

Example

To make it concrete my proposal would change Project.toml as follows:

before

name = "ChainRules"
uuid = "082447d4-558c-5d27-93f4-14fc19e9eca2"
version = "1.61.0"

[deps]
Adapt = "79e6a3ab-5dfb-504d-930d-738a2a938a0e"
ChainRulesCore = "d360d2e6-b24c-11e9-a2a3-2a2ae2dbcce4"
GPUArraysCore = "46192b85-c4d5-4398-a991-12ede77f4527"

[compat]
Adapt = "3.4.0, 4"
ChainRulesCore = "1.20"
ChainRulesTestUtils = "1.5"
FiniteDifferences = "0.12.20"
GPUArraysCore = "0.1.0"
julia = "1.6"

[extras]
ChainRulesTestUtils = "cdddcdb0-9152-4a09-a978-84456f9df70a"
FiniteDifferences = "26cc04aa-876d-5657-8c51-4c34ba976000"
JLArrays = "27aeb0d3-9eb9-45fb-866b-73c2ecf80fcb"
Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
StaticArrays = "90137ffa-7385-5640-81b9-e52037218182"
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"

[targets]
test = ["ChainRulesTestUtils", "FiniteDifferences", "JLArrays", "Random", "StaticArrays", "Test"]

after

name = "ChainRules"
uuid = "082447d4-558c-5d27-93f4-14fc19e9eca2"
version = "1.61.0"

[deps]
Adapt = "79e6a3ab-5dfb-504d-930d-738a2a938a0e"
ChainRulesCore = "d360d2e6-b24c-11e9-a2a3-2a2ae2dbcce4"
GPUArraysCore = "46192b85-c4d5-4398-a991-12ede77f4527"

[compat]
Adapt = "3.4.0, 4"
ChainRulesCore = "1.20"
ChainRulesTestUtils = "1.5"
FiniteDifferences = "0.12.20"
GPUArraysCore = "0.1.0"
julia = "1.6"

[test]
ChainRulesTestUtils = "cdddcdb0-9152-4a09-a978-84456f9df70a"
FiniteDifferences = "26cc04aa-876d-5657-8c51-4c34ba976000"
JLArrays = "27aeb0d3-9eb9-45fb-866b-73c2ecf80fcb"
Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
StaticArrays = "90137ffa-7385-5640-81b9-e52037218182"
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"

diff

18c18
< [extras]
---
> [test]
25,27d24
< 
< [targets]
< test = ["ChainRulesTestUtils", "FiniteDifferences", "JLArrays", "Random", "StaticArrays", "Test"]
vchuravy commented 5 months ago

A counter argument to this is that folk should stop using the [extras]/[targets] section and use test/Project.toml. However myself and I know many others actually prefer everything in one file -- makes it harder to mistakenly do funny things with [compat].

One of the reasons why I strongly prefer test/Project toml is that it clearly separates the meaning of [compat]. Both with the proposed scheme and the previous [extras] there is a UX ambiguity. Do compat bounds to dependencies in [extras]/[test] impact resolution of a package during normal installation?

For some users the answer may be clear: Of course it doesn't those compat bounds are only applied when tests are instantiated. For other users this may not be clear and can lead to confusion, and extra cognitive overhead when needing to check if a stated compat bound is applicable (is it just a test dependency?)

I believe the original design for [extras] was that there could be more than one test target, but that use case never materialized.

Overall I am not yet convinced that the pain of migration is worth the extra effort (and I personally would prefer going all in on test/Project.toml)

IanButterworth commented 5 months ago

Note that you can have both test and build targets

oxinabox commented 5 months ago

Huh, I never knew that. I think that makes this a nonstarter. I guess that's rarely used since BinaryProvider kind of overtook it. Especially when it became a stdlib in 1.3