-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Fix non-portable distroRID generation on Alpine Linux #62942
Conversation
Tagging subscribers to this area: @dotnet/area-infrastructure-libraries Issue Detailsinit-distro-id.sh generates incorrect DistroRid on Alpine. While the expected DistroRid (alpine-x.xx-xxx) should only include macro version, init-distro-id.sh includes the micro version. This patches it to cut off the trailing subversion off of VERSION_ID by treating it the same way RHEL's VERSION_ID is treated. Made as part of Alpine Linux dotnet31 / dotnet5 packaging project, see dotnet/source-build#2695
|
What is a benefit of a non-portable build? I am successfully using linux-musl-x64 official binary package (as well as custom builds) on Alpine and Void musl for past couple of .NET / .NET Core releases. Same goes for glibc based distros (with generic rid: linux-x64). re #62184 (comment); non-portable build is a legacy concept from .NET Core 1.x days and official builds stopped using it later (circa 2017). I'm sure we do not want to feed the entire https://distrowatch.com/ database in our code. :) For the version warnings, compat boundaries, the distro package manager can decide relationships between OS and .NET versions and if/when someone will try to force install a latest package on a very older version of OS, they will get exactly the same behavior as from any other software package manager is providing (may break, may work, no guarantees). |
Thanks for asking! I'm currently working on the package for dotnet3.1, dotnet5 and dotnet6 for Alpine Linux's aport. There were a few factors that informed my going the non-portable build route for the APKBUILD script.
With that said, I made the pull request as a courtesy in case this was indeed a bug for non-portable builds. I understand that the runtimes / sdk binaries made available by Microsoft should absolutely be portable. Although I am concerned that, without this patch (at least within the build environment for 5.0.209-SDK on Alpine Linux 3.15), non-portable builds do not work. (edit: oh and I get the distrowatch concern, haha. Makes sense to not want to create if statements for every thing out there. I'm understanding the portable approach now given that. Although, it does beg the question, why update RID for alpine at all?) |
This is a very good question, and there are some on-going discussions on this topic to rethink this facility: #59803 and NuGet/Home#5862. However, as it stands (and for past couple of releases, predating the least supported version: 3.1), the corehost (
If it doesn't find any matching nodes, it will bail and error out. Given that, portable variant is more permissive/flexible. The rest of the "compatibility" concern is not a huge issue (empirically speaking), as $other software in aports, debian packages etc. also have similar characteristic that if we force install package from other version lane, there are no guarantees that it will work (you will have to manually download/arrange dependencies for your application; which also is not super inconvenient these days). More concrete example could be made that a third-party nuget package has a dependency on some version of a lib, which is not available on certain distro version X, then the consumer will get a runtime error, instead of a package restore ( However, in practice, at least I have not seen users running into this kind of compat issues with official builds; where existing tooling leave them clueless (again: official builds, which have significant consumers, are portable for past several releases). |
Truth be told, I'm not a .NET programmer: I saw an important hole in Alpine's packages, and decided to fill it by writing an APKBUILD, so I appreciate that your sharing your knowledge. If you'll allow me to pick your brain a bit more, I see that portable builds offer more flexibility for nuget packages, thus might be more convenient for the end-user. In trying to figure out best practices, I looked at the packages on Arch Linux and Fedora, both use /p:SkipPortableRuntimeBuild as true, thus going the non-portable route. Why do you think they would go that route, given the by-design inflexibility of non-portable? |
I think it was added for .NET Core v1.x compatibility in the olden days and then it became a tech debt in runtime build configurations, which we are dragging along. e.g. all the modern versions of Ubuntu (after 16.04) do not have .NET Core 1.x compatible official package: https://github.com/dotnet/sdk/blob/1814193188014a50aa63d7c2c105336c3a0b6b93/eng/restore-toolset.sh#L23 (and all runtime releases afterwards was made distro-agnostic / portable which is working out just OK; it may have lost some pedantic compatibility warnings here and there but that has not risen to the level of "critical issue"). I haven't find the arguments to the contrary; what breaks if Arch and Fedora (also) switch to portable builds.. I personally consider From your perspective, I think the question boils down to: in a long run, would you rather update |
Thanks for putting it that way. Indeed, having to update RID at every update strikes me as odd. Okay, I'm sold. I'll go ahead and try a portable build, see how my APKBUILD fairs. This should slim down my patchset, which for maintenance is going to be less headache for me and future maintainers. This would make this merge request unncessary, if future best practice is to move away from non-portable altogether. |
I'm seeing some dissent on this being future best practice in this thread: dotnet/source-build#1529. Hard to figure out what / why people are attached to non-portable RIDs given your above arguments. As a practice, it seems desirable by package maintainer because of a perceived "protection", I suppose. But then why should dotnet do the package manager's job at making sure dependencies are there? A technicaly question for you: Are the non-portable binaries optimized for that build environement while portable bits aren't for sake of portability? Or do the differences stop at a differing compatibility matrix as enumerated above? |
This. From the perspective of dotnet/runtime and its dependencies, AFAIK, there aren't any particular optimizations we can achieve from non-portable build, which otherwise are not possible from portable builds. e.g. one of the coreclr dependency is libunwind. In v1.x, it was required from the platform, but later in the effort to reduce the dependency requirements, it was copied in-tree and eversince it is compiled with coreclr in builds from .NET Foundation. Some distros requested and added a build-time switch to continue using this dependency from the platform, because this library is customized/optimized differently in their ports. That switch can still be used with portable builds and is not tied to non-portable build. Another scenario where non-portable builds do not have a real benefit over portable ones:
(fix in both cases is typically either to update that lib on old version either from OS package manager, or make-install that lib from source) If we now create a non-portable package, it will not behave any differently in this situation and developer will go through the same experience. Where would non-portable build make a difference:
(if I am a nuget package author who really cares that much about a good restore-time compatibility warning or error message, I'd opt for a post-restore "system requirement validation" mechanism using an msbuild target) So the "benefit" of using non-portable build is little and it depends on too many "what if"s that it is virtually non-existent. |
Super interesting, thanks! Further exchanges with @dagood and @crummel on dotnet/source-build#2695 parallels your explanation of portable vs non-portable vis-à-vis dynamic linking. They consider this a maintenance optimization, as it allows for automatic scanning of dependencies. Since the source-build team has identified non-portable builds as a high priority, I'm going to keep Alpine builds non-portable to follow that standard. Fedora and Red Hat package maintainers know no doubt much more than me, so I'll refer to them. Once I fix the odd kinks in the portabe build, I'll package them along-side in the bootstrap packages for effective N-1 => N builds between distro versions. Apparently, ASP.NET also relies on portable bits so it's necessary work. |
First, "use lib X from distro package" mechanism is very limited; one (HP libunwind) out of five external dependencies used by .NET Runtime has that option in build infrastructure. Second, it is not tied to portable vs. non-portable builds. |
Tagging subscribers to this area: @dotnet/runtime-infrastructure Issue Detailsinit-distro-id.sh generates incorrect DistroRid on Alpine. While the expected DistroRid (alpine-x.xx-xxx) should only include macro version, init-distro-id.sh includes the micro version. This patches it to cut off the trailing subversion off of VERSION_ID by treating it the same way RHEL's VERSION_ID is treated. Made as part of Alpine Linux dotnet31 / dotnet5 packaging project, see dotnet/source-build#2695
|
@ericstj There are two perspectives on this: that of Microsoft, which am11 has brought to the table, and that of package maintainers. This fix isn't needed for Microsoft's use case where portable builds are produced. From a package maintainer's perspective, this is necessary as, without it, non-portable builds fail on Alpine Linux for dotnet 3.1, 5.0 and 6.0. The source-build team has shared with me that it is a priority for them to maintain portable builds. This leads me to want to refer to them when proposing that we merge, regardless of the merits (or lack thereof) of non-portable builds. Equally important to note: the backported version for 3.1 (dotnet/coreclr#28227) has already been approved for merge. One might fear that this sets a precedent where we might be tempted to include a bunch of systems within this if statement. I think that's a non-issue given that Alpine Linux portable builds are officially produced by Microsoft. In my view, this should strengthen the case to support non-portable builds by upstream on this system. |
@ericstj, my understanding is that non-portable build of runtime entails hardcoding OS version information without any real-world advantage over portable builds. If that is true, we should discourage non-portable builds of runtime at least for new distro packages (like aports in this case).
Portable build of runtime should not imply build from Microsoft and non-portable, the source-build. If this assumption breaks something in portable source-build, we should fix that. All the build knobs (turn feature X on and Y off) in runtime repo work with portable and non-portable builds alike.
This condition + host changes for every distro which has slightly different version string + runtime.compatibility.json updates for every version of each distro multiply it by every distro in the database ... then we see the scaling problem. This issue was realized in the light of the fact that no other programming platform cares about distro+version level of detail in build and there is no real-world use-case, which is why portable build mechanism (i.e. relaxed host and nuget package resolver) was implemented and made production-ready in .NET Core 2 timeframe. |
@hoyosjs -- can you have a look here since you approved dotnet/coreclr#28227 |
My take on this was:
|
@dotnet/source-build-internal, FYI in case the broader team is not aware of this thread. I think what's missing here is a complete accounting of why Red Hat and Fedora need the non-portable build to continue working. (So new distros can decide if it applies to them, too.) |
My take on this is that I think it's a good thing to reduce the build complexity and the benefits from producing distro-specific builds are minimal, if present at all. I think we should try to phase out distro-specific builds entirely. I would very much appreciate any existing user of the distro-specific builds to point out what problems this would cause. cc @dleeapho |
@janvorli is a long-time expert in the field of Unix distros, it might be interesting to hear his thoughts. |
I think that when dotnet is distributed as part of specific distro itself, the non-portable build makes sense, since that's the way that all the other packages that are part of the distro are built. The packages are supposed to depend only on the versions of packages published in the repo version. That enables the package manager of the specific distro to install all the necessary dependencies automatically and ensure that those versions work correctly with .NET. That would not work with portable build that has some of its dependencies detected dynamically at runtime. I believe that's why Redhat is building non portable builds. For packages that Microsoft builds though, I don't see any benefit of producing non-portable builds. |
@janvorli, do you know offhand which dependency loads differently just because of portable/non-portable difference? I don't see that in the code. |
@am11 I meant openSSL (FEATURE_DISTRO_AGNOSTIC_SSL). I have thought that we still do have that mechanism for ICU, but I have found that it was removed in the past so that we always dynamically load it. |
Afaik we aren't discussing portable vs non-portable build configuration. We're discussing the value of the non-portable rids. Feedom as in FOSS means you can build stuff from source. Yes, Microsoft binaries run everywhere. That shouldn't stop us from building something ourselves. But building something ourselves shouldn't stop users from using portable artifacts provided by Microsoft.
I don't understand what you mean by this: we build .NET for Fedora 35 on Fedora 35. That we use the non-portable rid That
This is also about build configuration. I agree, each build can chose its own flags. |
@tmds Unfortunately, I still need this patch to build under 6.0, although I don't add the missing Rid using your method. If my reading of that PR is correct, adding |
This PR is related to non-portable build. RID is implicitly related and it is calculated at run-time instead of build-time.
It sounds cool, but I don't see how does that affect our ability of building .NET "from source", "by ourselves"?
It sounds like a real problem, but data shows us it's not #62942 (comment). NuGet package published by Microsoft, Fedora, third-party org, individual etc. all kinds of possibilities cannot be all captured by the RID. It is insufficient for that purpose.
We shouldn't need to hard code distro info into the build artifacts and rely on platform relationships as if they will help solve compatibility issues. This is not normal. That is what I mean by on vs. for. |
@am11 Unless I'm mistaken of the use of I do appreciate the logic of avoiding hardcoding distro information in code, but so far I havn't found another solution to building dotnet on Alpine Linux without using this patch. I've intuited from @tmds that a solution might be to set |
At least it was designed to support both flavors https://grep.app/search?q=SourceBuildPortable&filter[repo][0]=dotnet/runtime. If there is a bug somewhere, we should try to fix it (mainly to avoid maintaining dead code). Thank you for your patience. I hope it was not blocking you from progressing, given both source-build and aports have source patching mechanism. |
@am11 Thanks for the flag, I didn't know about it., I'll do some tests on my side with it. No worries, indeed APKBUILD has a patching mechanism, thus this hasn't blocked me at all. I sent this PR as a courtesy if it was indeed bug. In addition, I've found the conversation really interesting, it certainly has taught me a lot of the backend in case I encounter any future issues. |
It enables distributing any package that Microsoft builds for the portable rid without making the portable rid non portable.
We're looking specifically at being able to build from source and distribute packages that Microsoft publishes under the portable rid. These packages provide key features for .NET such as SCD and crossgen. The design shouldn't prevent anyone other than Microsoft to distribute these features.
We shouldn't use
This should automagically happen by:
|
Does the build-from-source RID even end up anywhere? Nothing in dotnet/runtime, to my knowledge, ever cares about (e.g.) Fedora 34 vs Fedora 35... just AnyOS/Windows/Unix/Linux/macOS/iOS/tvOS/Android/Browser, at which point it's all just "linux" for a Fedora. If a distro is building to put things in their package manager, then the ELF files from the build won't go across distros or distro-versions (generally?), so the "what version of glibc was I linked against?" doesn't matter... that only matters for the As for the portable build vs individual feature flags... I don't think we're married to the idea of one big "I'm a distro" vs "I'm the semi-universal redist" switch... it was just an artifact of when we went for building on/for each distro individually to making the universal redist... it was the flag we needed to change to universal. I'm, personally, supportive of most anything that gets .NET into official distro package feeds. If a distro wants to officially put up a concrete request like "we want to keep ICU in flexible binding, but change the OpenSSL binding to classic dynamic linking" then that's probably something we can entertain... though not sure what level of resourcing is available, so it'd be one of those "PRs welcome" situations (provided that the manner of configuration is maintainable/understandable) -- but, as far as I can tell (with about 3 minutes' investigation), literally the only thing that varies on the portable build switch during compilation is |
@tmds Reconfirming build bug as of 6.0.102:
Missing file is actually in edit: Do note that I add RIDs for Alpine to |
This comment was marked as off-topic.
This comment was marked as off-topic.
In the host that lives at And imo we should also consider the packages Microsoft publishes for the portable rid (like
The difference is |
This change is appropriate to match the normalization that happens in other places like: runtime/src/native/corehost/hostmisc/pal.unix.cpp Lines 718 to 736 in a4a6d30
@ericstj can you merge the PR? The discussion about the sense/non-sense of non-portable rids can be continued on NuGet/Home#5862, #59803 or a new issue. |
/azp run runtime |
Azure Pipelines successfully started running 1 pipeline(s). |
I hit the retry button on the timed-out AzDo jobs. It sounds like we should move the discussion about portable RIDs to NuGet/Home#5862? I can post over there and summarize the discussion from this issue. |
This can be merged. The rid discussion has moved to the Simplify RID Model design. |
Ping. Can we merge the fix in this PR? I don't think it's controversial at all. I need it to get CI for alpine working. The RID discussion is worth continuing, but I don't think there's any need to hold back this PR; it's not adding or modifying RIDs to the graph. |
Yes, this is fine to merge. |
init-distro-id.sh generates incorrect DistroRid on Alpine. While the expected DistroRid (alpine-x.xx-xxx) should only include macro version, init-distro-id.sh includes the micro version. This patches it to cut off the trailing subversion off of VERSION_ID by treating it the same way RHEL's VERSION_ID is treated.
Made as part of Alpine Linux dotnet31 / dotnet5 packaging project, see dotnet/source-build#2695