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

Revisit the way packs work - always unpack them #645

Closed
nicolas-grekas opened this issue Jun 30, 2020 · 8 comments · Fixed by #656
Closed

Revisit the way packs work - always unpack them #645

nicolas-grekas opened this issue Jun 30, 2020 · 8 comments · Fixed by #656

Comments

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Jun 30, 2020

Packs create issues like symfony/orm-pack#25

For sure, we don't want to maintain versions of packs each time a dependent is bumped.
The current solution is like: use packs until it breaks for you, then unpack.

Instead, I'd like to propose the following:

  • keep using * as the version constraint for dependents in packs,
  • auto-unpack when installing a pack,
  • auto-replace the * by the current version - compute this "current version" by borrowing what composer does when running compose require without a version constraint (when the package is already found in the main composer.json, skip this),
  • additionally, also unpack require-dev,
  • (skip "conflict" and/or "replace", that's too much work, better consider them unsupported in packs, until one contributes the logic at least?).

Related to symfony/orm-pack#26, #370, #371, #563

@stof
Copy link
Member

stof commented Jun 30, 2020

For sure, we don't want to maintain versions of packs each time a dependent is bumped.

Why wouldn't we want to do that ? AFAIK, we don't have lots of packs involving non-Symfony packages (Symfony packages being covered by the special Symfony constraint of Flex).

Unpacking automatically in the require command when Flex is overriding it will still not prevent people from having such package in their requirement for another reason. If we keep an * requirement there, we would have to explicitly document that our pack packages are not following semver at all and are not covered by the Symfony BC promise.

@nicolas-grekas
Copy link
Member Author

Because there's zero value in doing so. We need to simplify our processes. I would dedicate zero minutes of my OSS time to maintaining this. And I'm sure that's the same for everyone.

semver/BC policy is about code, not about dependencies. That's a common misunderstanding.

What issues do you see in that proposal? Combined with flex (packs are a flex-only thing), resolving the * when unpacking has no drawbacks to me.

@stof
Copy link
Member

stof commented Jun 30, 2020

@nicolas-grekas semver is about the public API of your package, which might involve some APIs defined in dependencies (if you return one of their objects for instance). For a pack, the only API it has are the transitive dependencies, as that's what the pack is about.

@stof
Copy link
Member

stof commented Jun 30, 2020

packs are a flex-only thing

They are not. they are registered on Packagist as normal packages.

@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented Jun 30, 2020

There is no API in a pack. This interpretation you give is way too broad.

packs are a flex-only thing

Yes they are: "type": "symfony-pack". If one installs a pack and doesn't use flex, they're on their own.

@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented Jul 8, 2020

In #651, I made * be turned into regular constraints when unpacking.
In #652, I'm proposing to unpack recursively.

About unpack-by-default, there is a drawback: it will make uninstalling them impossible.
We need to decide if this a major flaw and if there is a way around before going this route.
An alternative option might be to make this configurable, via e.g. extra.auto-unpack or similar.
WDYT on this point?

About skipping conflicts, that might be unneeded actually: when packs are installed, the conflicts are taken into account. Now that #651 locks the major version of installed packages when unpacking, this kinda freezes the resolved conflicts. Of course, it's always possible that removing the conflicts will yield a different solution after unpacking, but the window becomes quite narrow. Maybe narrow enough to close #371 and save us from spending time on this.

For "replace", we should unpack them, see #653

Unpacking require-dev should be done separately, see #654

nicolas-grekas added a commit that referenced this issue Jul 14, 2020
This PR was merged into the 1.8-dev branch.

Discussion
----------

Auto-unpack on create-project

This PR ensures that packs are auto-unpacked when they are found in the `flex-require` or `flex-require-dev` sections, which happens when running e.g. `composer create-project symfony/(website-)skeleton`.

On the path to #645

Commits
-------

998d18f Auto-unpack on create-project
@fabpot fabpot closed this as completed in afd3d71 Jul 14, 2020
tgalopin pushed a commit to tgalopin/flex that referenced this issue Dec 3, 2020
@GPHemsley-RELX
Copy link

GPHemsley-RELX commented Jun 25, 2021

If packs are always unpacked, doesn't that mean that one wouldn't get the benefits of any future updates to the packs?

And wouldn't that include losing the benefit of pack-related recipes? For example, it seems that apache-pack's only purpose is to provide the connection to its recipe, as it has no dependencies.

@weaverryan
Copy link
Member

Hey @GPHemsley-RELX!

Yes, if we added new packages to a pack, then yes, users wouldn’t get the new package when updating. But that’s the lesser of 2 evils. Packs are basically a shortcut to helping installing a bunch of packages in the beginning.

About the pack recipes, the Apache one is an edge case (I’m actually not sure what happens when that unpacks). But generally-speaking, yes, pack recipes make less sense. We’ve moved away from them to attaching recipes more often to specific packages, instead of the pack itself.

cheers!

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 a pull request may close this issue.

4 participants