Add initial Linux platform and scripts #413

chrisd8088 · 2020-08-10T06:14:21Z

This is an initial port of the GVFS.Platform.Linux classes and the Scripts/Linux scripts from both the Mac equivalents in this repository as well as the corresponding classes in the VFSForGit repository.

Note that some useful refactoring which exists in the VFSForGit features/linuxprototype branch, such as microsoft/VFSForGit@7b70850, has not yet been ported; this would (re-)eliminate some of the duplication between the Mac and Linux platforms and move common code into the POSIX one.

This PR also does not include the case-sensitive filename support in VFSForGit's master branch from microsoft/VFSForGit#1412.

Quoting from the primary VFSForGit commit microsoft/VFSForGit@b304cf7 from which this work was derived:

    We add the core GVFS.Platform.Linux classes, derived from their
    GVFS.Platform.Mac equivalents but with appropriate changes for
    syscall argument signatures and flag values, type definitions,
    and structure fields (e.g., mode_t is a uint, and the layout of
    struct stat is quite different).

Running Scripts/Linux/RunUnitTests.sh on my system with .NET Core 3.1.302 reports:

Test Run Successful.
Total tests: 267
     Passed: 267

(With the still draft-only work in #422, all functional tests which aren't marked WindowsOnly or otherwise excluded on macOS also pass on Linux when using these initial platform classes. However, that points to some possible gaps in the test coverage, as case-insensitive path comparisons appear to not be causing test failures.)

kyle-rader · 2020-08-10T17:49:55Z

.azure-pipelines/pull-request.yml

+    pool:
+      name: ubuntu-latest
+    steps:
+      - powershell: Scripts/CI/Set-Version.ps1 -SourceBranchCounter $(branchCounter)


Does the PowerShell task #JustWork on linux agents?

Almost certainly not. :-/ I had a dim recollection that it somehow just worked everywhere from setting up Linux build pipelines for VFS4G, but looking over those again I see we actually used the Bash job in all the Linux pipelines. Will change, thanks!

I don't see it called out but I'm guessing since the Demand it makes is the DotNetFramework, that implies Windows? I asked since I know we now support Powershell on mac and linux, but I don't know what that requires :/

For now I'm just going to heavily lean on the GitHub Actions setup you created in git-ecosystem/git-credential-manager#157 as a slightly modified version seems to be just fine here. That simplifies the whole CI setup quite a bit and so for now I think I'll ignore any attempt to get things working in Azure.

Scalar.Common/Platforms/Linux/LinuxDaemonController.cs

derrickstolee · 2020-08-10T17:39:24Z

Scalar.Common/Platforms/Linux/LinuxFileBasedLock.cs

+
+namespace Scalar.Platform.Linux
+{
+    public class LinuxFileBasedLock : FileBasedLock


We will still need this for the scalar run verb, I think. Good to include.

derrickstolee · 2020-08-11T12:51:00Z

/azp run microsoft.scalar

azure-pipelines · 2020-08-11T12:51:11Z

Azure Pipelines failed to run 1 pipeline(s).

derrickstolee · 2020-08-11T12:55:57Z

@chrisd8088: I started the pipeline as currently-written, and got an error. I expect that you're currently writing GitHub Actions YAML instead, so perhaps that error doesn't matter.

.github/workflows/continuous-integration.yml

derrickstolee

Outside of the platform instance bug I found, I think this is an excellent first step! I'd be happy to merge in these results and start making the GitHub actions builds a required step for merging to main!

Or, do you still think this merits a [WIP] warning, @chrisd8088?

Scripts/Linux/Scalar_Clone.sh

Scripts/Linux/RunFunctionalTests.sh

derrickstolee · 2020-08-14T02:05:17Z

Scripts/Linux/NukeBuildOutputs.sh

@@ -0,0 +1,8 @@
+#!/bin/bash


This mimics scrips in the other directories, so this is fine.

Scripts/Linux/CleanupFunctionalTests.sh

Scripts/Linux/BuildScalarForLinux.sh

chrisd8088 · 2020-08-14T07:05:21Z

To be honest, I think the decision to merge or not is entirely up to you! I've been holding off while I experiment with running the functional tests (which break in all kinds of ways), but if you wanted to move forwards with this as it stands, I'm OK with that too -- but I would want to rewrite the commits somewhat at a bare minimum.

There will necessarily be refactoring of the Mac and Linux platforms to get them back to parity with the VFSForGit ones, where we'd made some progress eliminated duplicated code. Again, that's the sort of thing I thought I would try to get further along before proposing this as a "real" PR.

Let me see if I can clean this up a bit more, at least, tomorrow, if that works for you. I am happy with the CI builds in Actions, though, so that could also be a separate PR, I suppose.

This string was added when toying with testing SSH URLs in the functional tests. However, there was an issue with using SSH inside the build machines, so it was never used. It's now creating pain for the GitHub Actions workflows that will build Scalar. Drop it. (This was noticed by the warnings in #413.)

derrickstolee · 2020-08-14T12:49:24Z

To be honest, I think the decision to merge or not is entirely up to you!

You're absolutely right, which is why I asked what was still "in progress" as part of your "WIP" designation.

I've been holding off while I experiment with running the functional tests (which break in all kinds of ways), but if you wanted to move forwards with this as it stands, I'm OK with that too -- but I would want to rewrite the commits somewhat at a bare minimum.

Rewriting commits to satisfy your quality bar is definitely something you should take time to do.

I would prefer that we get a PR that runs the build + unit test step before we do the extra work to get functional tests working. Trying to do them all at once sounds like a recipe for PR bloat that will make it hard to review.

There will necessarily be refactoring of the Mac and Linux platforms to get them back to parity with the VFSForGit ones, where we'd made some progress eliminated duplicated code. Again, that's the sort of thing I thought I would try to get further along before proposing this as a "real" PR.

Please don't consider "parity with VFS for Git" a goal. Updating the platforms to reflect necessary platform differences is a valuable effort, but preferably that would be isolated to demonstrating that functional tests start to pass with those changes. The unit tests don't need these platform updates.

Let me see if I can clean this up a bit more, at least, tomorrow, if that works for you. I am happy with the CI builds in Actions, though, so that could also be a separate PR, I suppose.

Yes! I'm very happy to have the GitHub Actions workflows merged to the main branch while you continue to work on the hard platform-specific aspects of the Linux work. Just trying to pull out pieces that can be merged earlier than that.

chrisd8088 · 2020-08-14T17:51:28Z

OK, I can tweak these into two PRs in the next while, one for the basic first pass on the platform, and one for adding the Actions builds.

As for the functional tests, I'm just chipping away at problems, most of which so far have been config issues, but now seem to have something going sideways in FastFetch, but that too could be a config issue at its root. In brief, nothing too exciting to report there, just chipping at the coal face.

derrickstolee · 2020-08-14T19:47:43Z

As for the functional tests, I'm just chipping away at problems, most of which so far have been config issues, but now seem to have something going sideways in FastFetch, but that too could be a config issue at its root. In brief, nothing too exciting to report there, just chipping at the coal face.

FastFetch? Where is there still FastFetch code in scalar?

chrisd8088 · 2020-08-14T20:15:18Z

Yeah, don't take that too seriously -- I was just in the midst of debugging, and so far everything is purely config-related. In that case, FastFetch was turning up because it's part of the test repo in https://gvfs.visualstudio.com/ci/_git/ForTests which is still used in Scalar.FunctionalTests/Settings.cs as test fodder. Let me get to the point of some "real" failures before commenting in depth, which should be soon, I think.

chrisd8088 · 2020-08-14T21:08:30Z

Latest functional test run was actually pretty successful:
Test Count: 196, Passed: 190, Failed: 2, Warnings: 0, Inconclusive: 0, Skipped: 4.

derrickstolee

Please merge after #418 so we can run the PR validation here.

derrickstolee · 2020-08-18T01:19:06Z

I'm going to close and reopen so the PR validation runs again (hopefully).

This is an initial port of the GVFS.Platform.Linux classes and the Scripts/Linux scripts from both the Mac equivalents in this repository as well as the corresponding classes in the VFSForGit repository. Quoting from the primary VFSForGit commit microsoft/VFSForGit@b304cf7 from which this work was derived: We add the core GVFS.Platform.Linux classes, derived from their GVFS.Platform.Mac equivalents but with appropriate changes for syscall argument signatures and flag values, type definitions, and structure fields (e.g., mode_t is a uint, and the layout of struct stat is quite different). We also include all or portions of microsoft/VFSForGit@4ec09a8, microsoft/VFSForGit@7c3c4fa, and microsoft/VFSForGit@ef3201b. Co-authored-by: Ashe Connor <ashe@kivikakk.ee>

These checks allow us to detect compilation on systems which may not meet requirements for stat(2) struct field layout and __xstat64() libc ABI support on Linux. From microsoft/VFSForGit@5b1236c.

derrickstolee self-requested a review August 10, 2020 17:25

kyle-rader reviewed Aug 10, 2020

View reviewed changes

derrickstolee reviewed Aug 11, 2020

View reviewed changes

chrisd8088 force-pushed the linux-initial-trial branch 2 times, most recently from c4a0da8 to 7f22e53 Compare August 12, 2020 08:08

derrickstolee reviewed Aug 12, 2020

View reviewed changes

.github/workflows/continuous-integration.yml Outdated Show resolved Hide resolved

chrisd8088 force-pushed the linux-initial-trial branch from 7f22e53 to 04b18db Compare August 13, 2020 07:32

derrickstolee mentioned this pull request Aug 14, 2020

ScalarCloneFromGitHub: drop SSH string #417

Merged

derrickstolee reviewed Aug 14, 2020

View reviewed changes

chrisd8088 force-pushed the linux-initial-trial branch from 04b18db to 7d746ec Compare August 14, 2020 20:00

chrisd8088 force-pushed the linux-initial-trial branch 8 times, most recently from 589c6e2 to 75471c9 Compare August 17, 2020 06:39

chrisd8088 mentioned this pull request Aug 17, 2020

Linux: functional test and XDG configuration support #422

Merged

chrisd8088 changed the title ~~[WIP] [DO NOT MERGE] initial Linux platform and scripts~~ Add initial Linux platform and scripts Aug 17, 2020

chrisd8088 marked this pull request as ready for review August 17, 2020 07:09

derrickstolee approved these changes Aug 17, 2020

View reviewed changes

derrickstolee closed this Aug 18, 2020

derrickstolee reopened this Aug 18, 2020

chrisd8088 and others added 2 commits August 17, 2020 23:37

check libc and arch in Linux build

6614c18

These checks allow us to detect compilation on systems which may not meet requirements for stat(2) struct field layout and __xstat64() libc ABI support on Linux. From microsoft/VFSForGit@5b1236c.

chrisd8088 force-pushed the linux-initial-trial branch from 75471c9 to 6614c18 Compare August 18, 2020 06:39

chrisd8088 merged commit 562f127 into microsoft:main Aug 18, 2020

chrisd8088 deleted the linux-initial-trial branch August 18, 2020 07:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add initial Linux platform and scripts #413

Add initial Linux platform and scripts #413

chrisd8088 commented Aug 10, 2020 •

edited

Loading

kyle-rader Aug 10, 2020

chrisd8088 Aug 10, 2020

kyle-rader Aug 12, 2020

chrisd8088 Aug 13, 2020

derrickstolee Aug 10, 2020

derrickstolee commented Aug 11, 2020

azure-pipelines bot commented Aug 11, 2020

derrickstolee commented Aug 11, 2020

derrickstolee left a comment

derrickstolee Aug 14, 2020

chrisd8088 commented Aug 14, 2020

derrickstolee commented Aug 14, 2020

chrisd8088 commented Aug 14, 2020

derrickstolee commented Aug 14, 2020

chrisd8088 commented Aug 14, 2020

chrisd8088 commented Aug 14, 2020

derrickstolee left a comment

derrickstolee commented Aug 18, 2020

Add initial Linux platform and scripts #413

Add initial Linux platform and scripts #413

Conversation

chrisd8088 commented Aug 10, 2020 • edited Loading

kyle-rader Aug 10, 2020

Choose a reason for hiding this comment

chrisd8088 Aug 10, 2020

Choose a reason for hiding this comment

kyle-rader Aug 12, 2020

Choose a reason for hiding this comment

chrisd8088 Aug 13, 2020

Choose a reason for hiding this comment

derrickstolee Aug 10, 2020

Choose a reason for hiding this comment

derrickstolee commented Aug 11, 2020

azure-pipelines bot commented Aug 11, 2020

derrickstolee commented Aug 11, 2020

derrickstolee left a comment

Choose a reason for hiding this comment

derrickstolee Aug 14, 2020

Choose a reason for hiding this comment

chrisd8088 commented Aug 14, 2020

derrickstolee commented Aug 14, 2020

chrisd8088 commented Aug 14, 2020

derrickstolee commented Aug 14, 2020

chrisd8088 commented Aug 14, 2020

chrisd8088 commented Aug 14, 2020

derrickstolee left a comment

Choose a reason for hiding this comment

derrickstolee commented Aug 18, 2020

chrisd8088 commented Aug 10, 2020 •

edited

Loading