Skip to content

[HttpFoundation] Make dependency on Mime component optional #35642

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

Merged

Conversation

atailouloute
Copy link
Contributor

Q A
Branch? master
Bug fix? no
New feature? yes
Deprecations? no
Tickets
License MIT
Doc PR

Make the Mime component dependency optional

/cc @nicolas-grekas

@fabpot
Copy link
Member

fabpot commented Feb 8, 2020

Thank you @atailouloute.

fabpot added a commit that referenced this pull request Feb 8, 2020
…ional (atailouloute)

This PR was merged into the 5.1-dev branch.

Discussion
----------

[HttpFoundation] Make dependency on Mime component optional

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       |
| License       | MIT
| Doc PR        |

Make the Mime component dependency optional

/cc @nicolas-grekas

Commits
-------

11cef32 [HttpFoundation] Make dependency on Mime component optional
@fabpot fabpot merged commit 11cef32 into symfony:master Feb 8, 2020
@atailouloute atailouloute deleted the http-foundation-make-mime-optional branch February 8, 2020 12:59
@GrahamCampbell
Copy link
Contributor

I think this should be reverted. It breaks laravel/framework's 7.x branch which depends on symfony/http-foundation: ^5.0. When v5.1.0 comes out, it will be broken. I'd argue Laravel was not using symfony/mime implicitly, since it called code in http-foundation. It didn't make use of a non-direct dependency by bypassing http-foundation code. I think Symfony should wait until 6.0 before making this an optional dependency.

@GrahamCampbell
Copy link
Contributor

// cc @taylorotwell

@nicolas-grekas
Copy link
Member

Is Laravel 7 released? I'd suggest explicitly requiring symfony/mime on your side.
This change is planned for Symfony 5.1, which is unreleased also.
Note that this change is not covered by the BC policy. We already did so in the past and will continue to when needed.
Note also that in some specific cases, we decided to postpone the removal of some deps to the next major. This was done only for convenience. Here, I think you have plenty of time to adapt and you should. The change is easy enough to do.

@GrahamCampbell
Copy link
Contributor

Laravel 7 will depend on ^5.0, which means once 5.1.0 is tagged, we'd have unexpected breakage.

@GrahamCampbell
Copy link
Contributor

What was more concerning is that this change might be wanted in a symfony 4.4 patch release?

#35807 (review)

@nicolas-grekas
Copy link
Member

Even if you add symfony/mime in your deps explicitly?

@GrahamCampbell
Copy link
Contributor

We should be fine once we do that. My objection was to the principal of such changes.

@chalasr
Copy link
Member

chalasr commented Feb 23, 2020

What was more concerning is that this change might be wanted in a symfony 4.4 patch release?
#35807 (review)

I missed the PR that made it optional recently, sorry. This is for master.

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Feb 23, 2020

What was more concerning is that this change might be wanted in a symfony 4.4 patch release?

this won't happen, @chalasr must have been mistaken :)

We should be fine once we do that.

that's the way to go then, this change is desired on our side (to me at least.)

@GrahamCampbell
Copy link
Contributor

Ok. :)

nicolas-grekas added a commit that referenced this pull request Feb 23, 2020
…ahDude)

This PR was merged into the 5.1-dev branch.

Discussion
----------

[HttpFoundation] Fixed Mime dependency missing error

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | kinda
| New feature?  | no
| Deprecations? | no
| Tickets       | ~
| License       | MIT
| Doc PR        | ~

Follows #35642, by adding a missing exception and a note in the UPGRADE file (CHANGELOG in HttpFoundation was already up to date).
Reported in symfony/symfony-docs#1307

Commits
-------

fef0de3 [HttpFoundation] Fixed Mimes dependency missing error
@nicolas-grekas nicolas-grekas modified the milestones: next, 5.1 May 4, 2020
@fabpot fabpot mentioned this pull request May 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants