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

check for all-features with cargo-deny #10544

Merged
merged 2 commits into from Nov 14, 2023

Conversation

ameknite
Copy link
Contributor

@ameknite ameknite commented Nov 13, 2023

Objective

Fix #9880

Solution

@alice-i-cecile
Copy link
Member

On board with the updates, and the all-features. I'd prefer to avoid the MPL license here. It's LGPL-like IIRC. While I would be fine to use that in my commercial work, others might not be.

@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-Build-System Related to build systems or continuous integration C-Dependencies A change to the crates that Bevy depends on labels Nov 13, 2023
@ameknite
Copy link
Contributor Author

ameknite commented Nov 13, 2023

@alice-i-cecile the MPL-2.0 license is only copyleft if you modify the source files, is not like LGPL, so it should be fine if bevy only uses the crates and not modifies them.

from https://www.mozilla.org/en-US/MPL/2.0/FAQ/

Q11: How 'viral' is the MPL? If I use MPL-licensed code in my proprietary application, will I have to give all the source code away?

No. The license requires that Modifications (as defined in Section 1.10 of the license) must be licensed under the MPL and made available to anyone to whom you distribute the Source Code. However, new files containing no MPL-licensed code are not Modifications, and therefore do not need to be distributed under the terms of the MPL, even if you create a Larger Work (as defined in Section 1.7) by using, compiling, or distributing the non-MPL files together with MPL-licensed files. This allows, for example, programs using MPL-licensed code to be statically linked to and distributed as part of a larger proprietary piece of software, which would not generally be possible under the terms of stronger copyleft licenses.

@alice-i-cecile
Copy link
Member

I agree with your analysis, but I'd like to keep this uncontroversial :) Was there a reason to add this in the current PR?

@ameknite
Copy link
Contributor Author

ameknite commented Nov 13, 2023

Bevy uses Rodio, and Rodio uses Symphonia which is MPL-2.0 licensed.

If bevy wants to avoid the MPL-2.0 license. It will need to stop using rodio.

Symphonia is used through these flags in bevy_audio:

symphonia-aac = ["rodio/symphonia-aac"]
symphonia-all = ["rodio/symphonia-all"]
symphonia-flac = ["rodio/symphonia-flac"]
symphonia-isomp4 = ["rodio/symphonia-isomp4"]
symphonia-vorbis = ["rodio/symphonia-vorbis"]
symphonia-wav = ["rodio/symphonia-wav"]

@ameknite
Copy link
Contributor Author

The MPL-2.0 license is needed to fix #9880

  ┌─ symphonia 0.5.3 (registry+https://github.com/rust-lang/crates.io-index):4:12
  │
4 │ license = "MPL-2.0"
  │            ^^^^^^^
  │            │
  │            license expression retrieved via Cargo.toml `license`
  │            rejected: license is considered copyleft

@irate-devil
Copy link
Contributor

If bevy wants to avoid the MPL-2.0 license. It will need to stop using rodio.

Not necessarily. symphonia is an optional dependency of rodio, which is why it's only caught in this PR.

IMO rather than allowing MPL-2.0 generally we should add and exception for symphonia similar to unicode-ident for now and discuss symphonia's license in another issue (If it wasn't already discussed?).

@ameknite
Copy link
Contributor Author

ameknite commented Nov 14, 2023

yeah that could be a solution, but isn't this too verbose:

exceptions = [
    { name = "unicode-ident", allow = [
        "Unicode-DFS-2016",
    ] },
    { name = "symphonia", allow = [
        "MPL-2.0",
    ] },
    { name = "symphonia-bundle-flac", allow = [
        "MPL-2.0",
    ] },
    { name = "symphonia-bundle-mp3", allow = [
        "MPL-2.0",
    ] },
    { name = "symphonia-codec-aac", allow = [
        "MPL-2.0",
    ] },
    { name = "symphonia-codec-adpcm", allow = [
        "MPL-2.0",
    ] },
    { name = "symphonia-codec-pcm", allow = [
        "MPL-2.0",
    ] },
    { name = "symphonia-codec-vorbis", allow = [
        "MPL-2.0",
    ] },
    { name = "symphonia-core", allow = [
        "MPL-2.0",
    ] },
    { name = "symphonia-format-isomp4", allow = [
        "MPL-2.0",
    ] },
    { name = "symphonia-format-wav", allow = [
        "MPL-2.0",
    ] },
    { name = "symphonia-utils-xiph", allow = [
        "MPL-2.0",
    ] },
    { name = "symphonia-metadata", allow = [
        "MPL-2.0",
    ] },
]

I don't really see a problem why don't allow MPL-2.0 crates.

@MinerSebas
Copy link
Contributor

The potential problem is Console Support.
If that dependency requires changes for usage on a console, you may not share it due to an NDA, but that would conflict with the MPL License.
This is the reason that wgpu switched away from MPL.

So to be careful, it should be a case by case exception for dependencies that use MPL (When possible confirmed by a Dev with Console access).

@ameknite
Copy link
Contributor Author

ameknite commented Nov 14, 2023

@MinerSebas you are right, I didn't think about that. It could be a problem if we added a crate too intertwined with bevy.
I really dislike how consoles force you to put everything under an NDA, but well it is what it is.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Nov 14, 2023
@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Nov 14, 2023
Merged via the queue into bevyengine:main with commit 56d8a0e Nov 14, 2023
26 checks passed
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
# Objective

Fix bevyengine#9880

## Solution

- Add all-features flag 
- Allow "MPL-2.0" license for the
[Symphonia](https://github.com/pdeljanov/Symphonia) crates
- Update dependencies unmaintained or with vulnerabilities:
RustAudio/rodio#517 ,
LiquidityC/slice_ring_buffer#7
@ameknite ameknite mentioned this pull request Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Build-System Related to build systems or continuous integration C-Bug An unexpected or incorrect behavior C-Dependencies A change to the crates that Bevy depends on S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cargo deny doesn't check for all features
4 participants