https://bugzilla.redhat.com/show_bug.cgi?id=2142178
Bug ID: 2142178 Summary: Review Request: dotnet7.0 - .NET 7 SDK and Runtime Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: omajid@redhat.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://pagure.io/dotnet-sig/dotnet7.0/raw/main/f/dotnet7.0.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/%40dotnet-sig/dotnet-prev... Description:
.NET is a fast, lightweight and modular platform for creating cross platform applications that work on Linux, macOS and Windows.
It particularly focuses on creating console applications, web applications and micro-services.
.NET contains a runtime conforming to .NET Standards a set of framework libraries, an SDK containing compilers and a 'dotnet' application to drive everything.
Fedora Account System Username: omajid
https://bugzilla.redhat.com/show_bug.cgi?id=2142178
--- Comment #1 from Omair Majid omajid@redhat.com --- This is not an upgrade or replacement of the existing dotnet3.1 or dotnet6.0 packages, but an entirely new package. The packages can all be installed side-by-side.
Here's a diff of the dotnet6.0 and this new dotnet7.0 spec file: hhttps://gist.github.com/omajid/2912c6ceebd7445bc1d4419668cd15e1
This package needs to be bootstrapped. Which can be done by changing the %bcond_without. We have a blanket bootstrap exception from the fedora packaging committee: https://pagure.io/packaging-committee/issue/989.
https://bugzilla.redhat.com/show_bug.cgi?id=2142178
Neal Gompa ngompa13@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review? Status|NEW |ASSIGNED CC| |ngompa13@gmail.com Assignee|nobody@fedoraproject.org |ngompa13@gmail.com
--- Comment #2 from Neal Gompa ngompa13@gmail.com --- Taking this review.
https://bugzilla.redhat.com/show_bug.cgi?id=2142178
--- Comment #3 from Neal Gompa ngompa13@gmail.com --- Initial spec diff review:
* I'd prefer you kept the original changelog fully intact and added a new entry on top rather than blowing away chunks of the changelog * You should consider making a variable and using it everywhere that "7.0" is used so that it's easy to change and reduces the diff in the future (e.g. %dotnetver or something?) * Man pages are not guaranteed to have .gz extension (we could change to zstd compression in the future, for example), so use ".*" instead of ".gz" * Your conditional for the "dotnet" subpackage currently makes it get generated for RHEL < 9 *and* Fedora. Is that intentional? * Release "0.1%{?dist}" will create upgrade problems. Use "0%{?dist}.1" instead.
https://bugzilla.redhat.com/show_bug.cgi?id=2142178
--- Comment #4 from Omair Majid omajid@redhat.com --- Thanks!
Here's the updated package:
Spec URL: https://pagure.io/dotnet-sig/dotnet7.0/raw/main/f/dotnet7.0.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/%40dotnet-sig/dotnet-prev...
Copr build: https://copr.fedorainfracloud.org/coprs/g/dotnet-sig/dotnet-preview/build/50...
- I'd prefer you kept the original changelog fully intact and added a new entry on top rather than blowing away chunks of the changelog
I actually started working on this package a long time ago (probably around June). I couldn't propose to add it to Fedora back then because it wouldn't build and/or bootstrap until recently. The changelog diff is just a reflection of that. I can re-add the recent entries from dotnet6.0's changelog if you still prefer that.
- You should consider making a variable and using it everywhere that "7.0" is used so that it's easy to change and reduces the diff in the future (e.g. %dotnetver or something?)
- Man pages are not guaranteed to have .gz extension (we could change to zstd compression in the future, for example), so use ".*" instead of ".gz"
- Release "0.1%{?dist}" will create upgrade problems. Use "0%{?dist}.1" instead.
Thanks, all these should be fixed now.
- Your conditional for the "dotnet" subpackage currently makes it get generated for RHEL < 9 *and* Fedora. Is that intentional?
Kind of. Back when we added dotnet3.1 to Fedora, we had a `dotnet` subpackage to let users install the latest version of .NET/.NET Core. It was a mistake to have that because it's rarely what users want. Different versions of .NET aren't compatible with each other. Out of the box, a piece of code that wants .NET 6 will simply not work against .NET 7. So users generally want an explicit version of dotnet - and we should help them make that decision explicitly at install time. For RHEL 9, since it was a new OS, we could remove the dotnet subpackage without any issues. But we kept that subpacakge around in Fedora to keep different versions of .NET in different versions of Fedora consistent.
Is it okay if we leave this subpackage out Fedora starting with .NET 7? That means only dotnet6.0 (and earlier) will provide that package and users will get dotnet-sdk-6.0 installed if they ask for `dotnet`.
https://bugzilla.redhat.com/show_bug.cgi?id=2142178
--- Comment #5 from Neal Gompa ngompa13@gmail.com --- (In reply to Omair Majid from comment #4)
Thanks!
Here's the updated package:
Spec URL: https://pagure.io/dotnet-sig/dotnet7.0/raw/main/f/dotnet7.0.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/%40dotnet-sig/dotnet- preview/srpm-builds/05035862/dotnet7.0-7.0.100-0.fc36.1.src.rpm
Copr build: https://copr.fedorainfracloud.org/coprs/g/dotnet-sig/dotnet-preview/build/ 5035862/
- I'd prefer you kept the original changelog fully intact and added a new entry on top rather than blowing away chunks of the changelog
I actually started working on this package a long time ago (probably around June). I couldn't propose to add it to Fedora back then because it wouldn't build and/or bootstrap until recently. The changelog diff is just a reflection of that. I can re-add the recent entries from dotnet6.0's changelog if you still prefer that.
Oh, that makes sense. Don't worry about it then.
- You should consider making a variable and using it everywhere that "7.0" is used so that it's easy to change and reduces the diff in the future (e.g. %dotnetver or something?)
- Man pages are not guaranteed to have .gz extension (we could change to zstd compression in the future, for example), so use ".*" instead of ".gz"
- Release "0.1%{?dist}" will create upgrade problems. Use "0%{?dist}.1" instead.
Thanks, all these should be fixed now.
- Your conditional for the "dotnet" subpackage currently makes it get generated for RHEL < 9 *and* Fedora. Is that intentional?
Kind of. Back when we added dotnet3.1 to Fedora, we had a `dotnet` subpackage to let users install the latest version of .NET/.NET Core. It was a mistake to have that because it's rarely what users want. Different versions of .NET aren't compatible with each other. Out of the box, a piece of code that wants .NET 6 will simply not work against .NET 7. So users generally want an explicit version of dotnet - and we should help them make that decision explicitly at install time. For RHEL 9, since it was a new OS, we could remove the dotnet subpackage without any issues. But we kept that subpacakge around in Fedora to keep different versions of .NET in different versions of Fedora consistent.
Is it okay if we leave this subpackage out Fedora starting with .NET 7? That means only dotnet6.0 (and earlier) will provide that package and users will get dotnet-sdk-6.0 installed if they ask for `dotnet`.
It seems weird that we'd drop it though, given that we have the dotnet-host package generated too. I think it makes sense to have an unversioned package that gives you the latest one. That said, it does make sense to me that RHEL wouldn't have this anymore, given that you'll keep adding new versions across the same major RHEL version.
But if you really want to drop it, the conditional is not very clear on that. And all the other dotnet packages will need cleanup for that too...
https://bugzilla.redhat.com/show_bug.cgi?id=2142178
--- Comment #6 from Omair Majid omajid@redhat.com ---
It seems weird that we'd drop it though, given that we have the dotnet-host package generated too.
So `dotnet-host` provides an entry point (`/usr/bin/dotnet`) to running all .NET versions. Think of it like python's `venv` and `python3` all rolled into one command. dotnet-host also provides man pages and a bunch of other things. If we don't ship it we wont have .NET working at all. And we only really need to ship one version of that: the version from .NET 7 will handle .NET 6 just fine.
That said, it does make sense to me that RHEL wouldn't have this anymore, given that you'll keep adding new versions across the same major RHEL version.
I plan to do the same for Fedora. Fedora 36 has .NET Core 3.1 (dotnet3.1), .NET 6.0 (dotnet6.0) and I will be adding this .NET 7 (dotnet7.0) to Fedora 36 and later.
There just isn't a good reason for `dotnet` to exist as a separate package when everyone (including upstream) expects users to install versions explicitly.
But if you really want to drop it, the conditional is not very clear on that.
I can fix that shortly with better comments, if you want.
And all the other dotnet packages will need cleanup for that too...
Super long term, I do want to stop providing a plain unversioned `dotnet` package everywhere, including Fedora.
Would you be okay if I left dotnet3.1 and dotnet6.0 continue providing a `dotnet` package and just remove it from dotnet7.0? Or would you prefer we handle Fedora 38 and later differently than Fedora 37 and earlier?
https://bugzilla.redhat.com/show_bug.cgi?id=2142178
--- Comment #7 from Omair Majid omajid@redhat.com --- I updated the comment and conditionals for building the `dotnet` subpackage.
Spec diff: https://pagure.io/dotnet-sig/dotnet7.0/c/f78d53f5cc4f7ddbf05bbc491c6bb33d2d3... Spec URL: https://pagure.io/dotnet-sig/dotnet7.0/raw/main/f/dotnet7.0.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/%40dotnet-sig/dotnet-prev... Build: https://copr.fedorainfracloud.org/coprs/g/dotnet-sig/dotnet-preview/build/50...
https://bugzilla.redhat.com/show_bug.cgi?id=2142178
--- Comment #8 from Neal Gompa ngompa13@gmail.com --- (In reply to Omair Majid from comment #6)
It seems weird that we'd drop it though, given that we have the dotnet-host package generated too.
So `dotnet-host` provides an entry point (`/usr/bin/dotnet`) to running all .NET versions. Think of it like python's `venv` and `python3` all rolled into one command. dotnet-host also provides man pages and a bunch of other things. If we don't ship it we wont have .NET working at all. And we only really need to ship one version of that: the version from .NET 7 will handle .NET 6 just fine.
That said, it does make sense to me that RHEL wouldn't have this anymore, given that you'll keep adding new versions across the same major RHEL version.
I plan to do the same for Fedora. Fedora 36 has .NET Core 3.1 (dotnet3.1), .NET 6.0 (dotnet6.0) and I will be adding this .NET 7 (dotnet7.0) to Fedora 36 and later.
There just isn't a good reason for `dotnet` to exist as a separate package when everyone (including upstream) expects users to install versions explicitly.
But if you really want to drop it, the conditional is not very clear on that.
I can fix that shortly with better comments, if you want.
And all the other dotnet packages will need cleanup for that too...
Super long term, I do want to stop providing a plain unversioned `dotnet` package everywhere, including Fedora.
Would you be okay if I left dotnet3.1 and dotnet6.0 continue providing a `dotnet` package and just remove it from dotnet7.0? Or would you prefer we handle Fedora 38 and later differently than Fedora 37 and earlier?
I'd prefer F38+ differently from older versions. You'll probably want to fix up the developer.fedoraproject.org content for this new reality, though.
https://bugzilla.redhat.com/show_bug.cgi?id=2142178
--- Comment #9 from Omair Majid omajid@redhat.com --- Spec URL: https://pagure.io/dotnet-sig/dotnet7.0/raw/main/f/dotnet7.0.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/%40dotnet-sig/dotnet-prev... Spec diff: https://pagure.io/dotnet-sig/dotnet7.0/c/de2f31ceab56d5a754d031257301d856b89... Build: https://copr.fedorainfracloud.org/coprs/g/dotnet-sig/dotnet-preview/build/50...
https://bugzilla.redhat.com/show_bug.cgi?id=2142178
--- Comment #10 from Omair Majid omajid@redhat.com --- I will do a pass on developer.fedoraproject.org after this package is approved. I looked over it and it needs more changes than just `dotnet` to `dotnet-sdk-7.0` rename; it still talks about .NET Core 2.1 (which has been EOL for years) and dotnet-sig copr repos, which are unnecessary for all current Fedora releases.
https://bugzilla.redhat.com/show_bug.cgi?id=2142178
--- Comment #11 from Neal Gompa ngompa13@gmail.com ---
%patch1 -p1 %patch2 -p1 %patch3 -p1 %patch4 -p1 %patch5 -p1 %patch6 -p1 %patch7 -p1 %patch8 -p1 %patch9 -p1 %patch10 -p1 %patch11 -p1
Any reason you're not just using "%autopatch -p1" here?
https://bugzilla.redhat.com/show_bug.cgi?id=2142178
Neal Gompa ngompa13@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |182235 (FE-Legal)
--- Comment #12 from Neal Gompa ngompa13@gmail.com ---
License: MIT and ASL 2.0 and BSD and LGPLv2+ and CC-BY and CC0 and MS-PL and EPL-1.0 and GPL+ and GPLv2 and ISC and OFL and zlib
You might want to go ahead and convert this to SPDX notation now. Note that "CC0" is banned for code in Fedora nowadays, so if there's still CC0 code, you'll need an exception from FE-Legal.
Cf. https://docs.fedoraproject.org/en-US/legal/update-existing-packages/
Cf. https://lists.fedoraproject.org/archives/list/legal@lists.fedoraproject.org/...
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=182235 [Bug 182235] Fedora Legal Tracker
https://bugzilla.redhat.com/show_bug.cgi?id=2142178
--- Comment #13 from Omair Majid omajid@redhat.com ---
Any reason you're not just using "%autopatch -p1" here?
Thanks for catching this. We didn't use this in .NET 6 because we had to `pushd`/`popd` into the correct directory first before applying patches. But that's no longer needed.
Note that "CC0" is banned for code in Fedora nowadays, so if there's still CC0 code, you'll need an exception from FE-Legal.
It looks like only these code files have a mention of creative commons:
https://github.com/dotnet/runtime/tree/main/src/libraries/System.Private.Cor... https://github.com/dotnet/runtime/tree/main/src/libraries/System.Private.Cor...
https://github.com/dotnet/fsharp/tree/main/tests/benchmarks/CompiledCodeBenc... https://github.com/dotnet/fsharp/tree/main/tests/FSharp.Core.UnitTests/FShar... https://github.com/dotnet/fsharp/tree/main/tests/FSharp.Core.UnitTests/FShar...
https://github.com/dotnet/runtime/tree/main/src/mono/mono/utils/dlmalloc.c https://github.com/dotnet/runtime/tree/main/src/mono/mono/utils/dlmalloc.h
The first two have a MIT license header in addition to CC0. Is any action needed on those? Can I even "ignore" that they are under CC0 because of the MIT license header?
The next 3 don't have an MIT or Apache header. But they are tests (that aren't run during the build). I suppose I can just delete these files?
The final 2 are dlmalloc. This seems to be an older version of dlmalloc that uses the language that makes it CC-PDDC, which sounds acceptable to Fedora: https://bugzilla.redhat.com/show_bug.cgi?id=2120577#c1 ?
https://bugzilla.redhat.com/show_bug.cgi?id=2142178
--- Comment #14 from Omair Majid omajid@redhat.com --- Started a thread on legal@lists.fedoraproject.org : https://lists.fedoraproject.org/archives/list/legal@lists.fedoraproject.org/...
https://bugzilla.redhat.com/show_bug.cgi?id=2142178
--- Comment #15 from Omair Majid omajid@redhat.com --- While we wait for the discussion on the legal mailing list, are there any other issues we should resolve? Is this package okay to go as soon as the legal discussion is sorted out and we have some sort of approval?
https://bugzilla.redhat.com/show_bug.cgi?id=2142178
--- Comment #16 from Neal Gompa ngompa13@gmail.com --- No, I think we're in good shape after this. I'm holding off on granting the review until we have that figured out.
https://bugzilla.redhat.com/show_bug.cgi?id=2142178
--- Comment #17 from Omair Majid omajid@redhat.com --- As part of doing the conversion to SDPX, I ran into more issues:
Bugs in license-validate - WITH operator: https://pagure.io/copr/license-validate/issue/7 - BSD-4-Clause-UC: https://pagure.io/copr/license-validate/issue/8 - bzip2-1.0.6: https://pagure.io/copr/license-validate/issue/9 - LicenseRef-ISO-8879: https://pagure.io/copr/license-validate/issue/10
Missing SPDX identifiers in Fedora license list: - SAX-PD: https://gitlab.com/fedora/legal/fedora-license-data/-/issues/99 - W3C-19980720: https://gitlab.com/fedora/legal/fedora-license-data/-/issues/100
More questions, added to the existing thread on legal@ : https://lists.fedoraproject.org/archives/list/legal@lists.fedoraproject.org/...
https://bugzilla.redhat.com/show_bug.cgi?id=2142178
jarle@webmed.no changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |jarle@webmed.no
--- Comment #18 from jarle@webmed.no --- Is there any progress on this ?
https://bugzilla.redhat.com/show_bug.cgi?id=2142178
--- Comment #19 from Omair Majid omajid@redhat.com --- Spec URL: https://pagure.io/dotnet-sig/dotnet7.0/raw/main/f/dotnet7.0.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/%40dotnet-sig/dotnet-prev...
I am aware of 3 follow-up items:
- Add this MIT variant to SPDX and fedora-license-data: https://github.com/dotnet/runtime/blob/main/src/libraries/System.Text.Regula.... For now, I have worked with the assumption that this falls in the SPDX approach that MIT-is-a-category-of-licenses and just used MIT identifier for Fedora. - Add this MIT variant to SPDX/fedora-license-data: https://fedoraproject.org/wiki/Licensing:MIT?rd=Licensing/MIT#HP_Variant. Same as above. - https://gitlab.com/fedora/legal/fedora-license-data/-/issues/104#note_123064.... For now, I have ripped out that test code that copy/pasted from StackOverflow.
For CC0-1.0 in particular, the exception at https://gitlab.com/fedora/legal/fedora-license-data/-/blob/main/data/CC0-1.0... applies to .NET 7 since it's a newer version of .NET which was already included in Fedora (as dotnet3.1, and dotnet6.0, etc).
With that, I believe all legal issues have been sorted out. Is it okay to include this package in Fedora now?
https://bugzilla.redhat.com/show_bug.cgi?id=2142178
--- Comment #20 from Jakub Kadlčík jkadlcik@redhat.com --- Copr build: https://copr.fedorainfracloud.org/coprs/build/5206160 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
https://bugzilla.redhat.com/show_bug.cgi?id=2142178
Neal Gompa ngompa13@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |POST Blocks|182235 (FE-Legal) | Flags|fedora-review? |fedora-review+
--- Comment #21 from Neal Gompa ngompa13@gmail.com --- At this point, this package looks good to me now.
PACKAGE APPROVED.
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=182235 [Bug 182235] Fedora Legal Tracker
https://bugzilla.redhat.com/show_bug.cgi?id=2142178
--- Comment #22 from Gwyn Ciesla gwync@protonmail.com --- (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/dotnet7.0
https://bugzilla.redhat.com/show_bug.cgi?id=2142178
Omair Majid omajid@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|POST |CLOSED Resolution|--- |CURRENTRELEASE Last Closed| |2023-01-16 04:32:55
--- Comment #23 from Omair Majid omajid@redhat.com --- Thanks for the review and the approval!
I have built dotnet7.0 for all currently supported Fedora versions:
Rawhide: https://bodhi.fedoraproject.org/updates/FEDORA-2023-0fee44158c Fedora 37: https://bodhi.fedoraproject.org/updates/FEDORA-2023-27a1022e1f Fedora 36: https://bodhi.fedoraproject.org/updates/FEDORA-2023-3d309b302f
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com