-
Notifications
You must be signed in to change notification settings - Fork 867
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
Adjustments for FreeBSD #3266
base: master
Are you sure you want to change the base?
Adjustments for FreeBSD #3266
Conversation
Third times a charm? I know that @wfurt and @jasonpugsley have both tried to get support added in with no luck. Anyways... This does build after ignoring errors for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome to see some progress here! There are a lot of places that are 'if we're on Linux, or FreeBSD, or macOS', which really mean 'if we're on some POSIX platform'. It's generally a good idea when writing portable software to start with the lowest common denominator and then add specialisations. It would be great to factor these out into an isPosix
check that is set for everything non-Windows. We can then special case thing for specific systems as required. For example, the case-sensitive filesystem check should be 'is POSIX and not macOS'. That way, the next person who comes along and wants to add support for NetBSD / OpenBSD / Solaris / Haiku / whatever just has to deal with the places where their platform diverges from POSIX, rather than cluttering up the code further.
I'd prefer "Unix-like platform" or something like that. After spending years of work in FreeBSD ports I learned that POSIX is actually a pretty strict term, which shouldn't be used so lightly. In the Linux world there are APIs that are claiming to be POSIX-compliant, but they actually have various subtle specifics that do not match FreeBSD. I believe, the checks should use either |
Without getting too into depth about this, if we want POSIX compliance we are going to have to define which revision of POSIX. The newest is, uh, this one? Which revision should be explicitly named somewhere too While |
I agree with both of you, @arrowd and @Thefrank: public static class Capabilities
{
public static bool IsCaseSensitive
{
get => RuntimeInformation.IsOSPlatform(OSPlatform.Linux) || RuntimeInformation.IsOSPlatform(OSPlatform.FreeBSD);
}
public static bool SupportsSystemD
{
get => RuntimeInformation.IsOSPlatform(OSPlatform.Linux);
}
...
} We already know capabilities in another context from Azure Pipelines Agent, so why not use this concept in source code as well? However, this sounds like a useful but also big change that would blow up this PR. Maybe we can keep an eye on this in the future and create a separate PR for it? |
8bb5146
to
52aae17
Compare
2686cb5
to
53e946d
Compare
Any update on this? |
Everything is ready from my side. Do you have this PR in view, @davidchisnall and @wfurt? |
All of my comments are addressed and I'm happy but I'm not a maintainer on (or even contributor to) this repo, just a bystander with a vested interest in seeing Azure Pipelines support FreeBSD. |
dc2e44c
to
2c67a1f
Compare
2c67a1f
to
fe93612
Compare
fe93612
to
08bb302
Compare
its been about 5 months. I think this is good for another poke. |
@joperator The item it was processing:
The Error:
|
@Thefrank
However, your exception does not come directly from the Azure Pipelines Agent itself, but from the referenced cc: @wfurt |
yah. I think the relevant code lives outside of this repo. I'm yet to trace the real source. |
|
Yes, but keep in mind that these are two errors in two different places:
|
Patching The file looks like it comes from |
Any updates on fixing this issue in Microsoft.VisualStudio.Services.Content.Common.dll, @wfurt? |
08bb302
to
0209f60
Compare
0209f60
to
74d3060
Compare
bb8c3c6
to
056c445
Compare
056c445
to
2e4d0e5
Compare
2e4d0e5
to
6bd8100
Compare
Thanks for maintaining this PR! Also: Happy 2yr anniversary to this PR! |
6bd8100
to
3191649
Compare
3191649
to
2c9a9af
Compare
2c9a9af
to
a14b6ec
Compare
Getting close to 3yr :) |
a14b6ec
to
5a38db8
Compare
@sec With the lang/dotnet port for FreeBSD in hand, perhaps someone is more willing to merge this and microsoft/azure-pipelines-task-lib#799 before the big party starts? |
5a38db8
to
2c42a6f
Compare
2c42a6f
to
8ef90b7
Compare
8ef90b7
to
797e6c3
Compare
New tests that need skipping as they are also skipped on osx and linux:
This should give a clean test. |
797e6c3
to
1f57a3b
Compare
@Thefrank Thanks for your advice. |
You might want to add |
src/Misc/externals.sh
Outdated
if [[ "$INCLUDE_NODE10" == "true" ]]; then | ||
acquireExternalTool "${CONTAINER_URL}/nodejs/${ARCH}/node-v${NODE10_VERSION}-${ARCH}.tar.gz" node10/bin fix_nested_dir | ||
if [[ "$INCLUDE_NODE6" == "true" ]]; then | ||
linkExternalTool /usr/local/bin/node node/bin node |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The /usr/local
part should not be hardcoded but computed from sysctl:
LOCALBASE=$(sysctl -n user.localbase)
linkExternalTool "${LOCALBASE}/bin/node" node/bin node
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The /usr/local
part is now computed from sysctl
.
@Thefrank |
@joperator oops! my mistake was saying
SHORT: if the (too?) LONG: This was only discovered with FreeBSD 14.0 as earlier versions seem to have behavior inconsistent with the documentation and code. if anything from Under FreeBSD 13 these programs had no issues despite the documentation stating that these additional Bug from AFAIK This (FreeBSD 14.0) version is the correct behavior as it matches the code and documentation. |
@Thefrank |
1f57a3b
to
27e80b4
Compare
I am trying to get this to build and run recently, where would be the feed containing Microsoft.NETCore.App.Runtime.freebsd-x64...*.nupkg thanks in advance |
@littleq0903 |
I also try to maintain up-to-date private feed you can use |
Although not yet officially supported, there's still a need for Azure Pipelines Agent for FreeBSD, as described here.
This PR contains adjustments to build and run Azure Pipelines Agent on FreeBSD:
.cs
and.sh
files are made to account for FreeBSD where Linux is already accounted for.DaemonControlManager.cs
for FreeBSD, according to the control managers for other platforms.Common.props
is already supported by dotnet CLI, but not by Visual Studio, causing all projects that importCommon.props
to fail to load witherror : Invalid static method invocation syntax: "[System.Runtime.InteropServices.OSPlatform]::FreeBSD". Method 'System.Runtime.InteropServices.OSPlatform.FreeBSD' not found.
This should be fixed by Microsoft at some point.
daemon.svc.sh.template
andvsts.agent.daemon.template
to run Azure Pipelines Agent as a daemon on FreeBSD, since systemd is not supported on this platform.With this PR you should be able to build Azure Pipelines Agent on FreeBSD using the following workflow:
pkg install bash curl git icu libinotify libunwind node
src/dev.sh
changeDOTNETSDK_VERSION
to the version of the .NET SDK that you're using on FreeBSD.The .NET SDK must be able to create self-contained apps for FreeBSD.
src/Common.props
changeRuntimeFrameworkVersion
to the version of the NuGet packageMicrosoft.NETCore.App.Runtime.freebsd-x64
that is available to you.Microsoft.NETCore.App.Runtime.freebsd-x64.*.*.*.nupkg
to allNuGet.Config
files in the repo._layout/freebsd-x64
.