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

Port to .NET Core 3.0 and revamp engineering/build/test #201

Merged
merged 24 commits into from
Nov 7, 2019
Merged

Port to .NET Core 3.0 and revamp engineering/build/test #201

merged 24 commits into from
Nov 7, 2019

Conversation

mjcheetham
Copy link
Member

Non-goals

  • Signing

New commands
dotnet build: Build all managed code
dotnet test: Run all managed unit tests
dotnet publish -r [osx-x64|win-x64]: Build all code including Scalar installer packages and scripts

Scripts
BuildScalarForMac.sh/BuildScalarForWindows.bat: Build all code and generate installers and install scripts
RunUnitTests.(sh|bat): Run the managed unit tests
RunFunctionalTests.(sh|bat): Run the functional tests
RestorePackages.bat: No-op

Directories

.
├── out                          (build output)
│   ├── <Project1>
│   ├──    ...
│   ├── <ProjectN>
│   │
│   ├── Scalar.Distribution.Mac  (distribution: packages, scripts)
│   │  └── dist
│   │      └── <Configuration>
│   │          ├── InstallScalar.sh
│   │          │
│   │          ├── Scalar
│   │          │   └── Scalar.<version>.pkg
│   │          ├── GCM
│   │          │   └── gcmcore-osx-<version>.pkg
│   │          └── Git
│   │              └── git-<version>.pkg
│   │
│   └── Scalar.Distribution.Windows
│      └── dist
│          └── <Configuration>
│              ├── InstallScalar.bat
│              │
│              ├── Scalar
│              │   └── Scalar.<version>.exe
│              └── Git
│                  └── Scalar.<version>.exe
│
└── src                          (source)
    ├── <Project1>
    ├──    ...
    ├── <ProjectN>
    │
    ├── Dependencies.props       (Unified NuGet package versions)
    └── Directory.Build.props    (Common build props, including minimum Git version)
    └── Directory.Build.targets  (Common build target)

Changes

  • Functional tests drops in CI do not include copies of the product; the installer drop must be consumed and installed.
  • The metapackage on macOS has been dropped.. we only build the Scalar package and leave installation of the correct version of Git to the outside.
  • The Platform projects have been folded into the Common project, under the Platforms/ directory. There is now one platform loader that instantiates the correct type based on the current OS at runtime.
  • The Scalar.Build project has been slimmed down massively; we only need:
    • Generate Scalar constants (version)
    • Compile templated file (x-plat replacement for sed to replace placeholder tokens)
    • Generate Windows application manifest (set supported Windows versions)
  • The Scalar.Mac and Scalar.Windows projects have been folded into one, with a runtime fork to create the correct platform-specific types. The same applies for the other .Mac/.Windows projects.
  • Scalar.Tests has been renamed to Scalar.TestInfratructure to better reflect its purpose.
  • The Scalar.Notifications/Scalar.Mac directory has been flattened, and an MSBuild project been added that wraps the xcodeproj project. This allows us to chain in an xcodebuild into the MSBuild build with the dotnet CLI (see dotnet build/publish).
  • Introduce shims for some missing APIs in .NET Core from .NET Framework, mainly things around ACLs which have been implemented in the new type DirectoryEx in the Common project.
  • Include a copy of internal NamedPipeServerStream methods from the CoreFX to create pipes with the appropriate ACLs on Windows.

@mjcheetham mjcheetham marked this pull request as ready for review October 29, 2019 13:19
@mjcheetham mjcheetham changed the title [WIP] Port to .NET Core 3.0 and revamp engineering/build/test Port to .NET Core 3.0 and revamp engineering/build/test Oct 29, 2019
@derrickstolee derrickstolee added this to the MVP milestone Oct 29, 2019
@mjcheetham
Copy link
Member Author

Currently builds fail intermittently on Windows with NuGet/Home#8692

@mjcheetham
Copy link
Member Author

Currently builds fail intermittently on Windows with NuGet/Home#8692

Looks like a NuGet bug that was fixed in 5.1.
NuGet/Home#7854

Need to update the version of NuGet we're using in Pipelines.

@derrickstolee
Copy link
Contributor

I checked out your code on my Windows machine. Building with Scripts/BuildScalarForWindows.bat worked like a charm. Visual Studio opened the solution just fine. Test Explorer sees the unit tests AND the functional tests. I only tried running the unit tests, which worked (except for tests that use [TestCaseSource(...)] instead of [TestCase]).

I've re-cloned the Office repo using this branch and am currently hydrating files using the gvfs-helper. I'll double-check the maintenance steps will start running from the service.

@derrickstolee
Copy link
Contributor

@mjcheetham: I had @wilbaker complete his PR, but didn't realize the conflicts it would cause for you. The plan is to hold off on further major changes until this is ready, unless we think it's going to take a long time. For instance, William isn't going to delete the Scalar.Mount project until this is done, but he will be working on some logic in the service.

cc: @jrbriggs

@wilbaker
Copy link
Member

wilbaker commented Oct 30, 2019

@mjcheetham: I had @wilbaker complete his PR, but didn't realize the conflicts it would cause for you. The plan is to hold off on further major changes until this is ready, unless we think it's going to take a long time. For instance, William isn't going to delete the Scalar.Mount project until this is done, but he will be working on some logic in the service.

Yep apologies for the conflicts! Fortunately they are easy to resolve.

procedure CurStepChanged(CurStep: TSetupStep);
begin
  case CurStep of
    ssInstall:
      begin
        UninstallService('Scalar.Service', True);
      end;
    ssPostInstall:
      begin
        InstallScalarService();
        RegisterUserWithService();
      end;
    end;
end;

If you're interested I pushed a branch here that has a merge of master into refactor3 with the above conflicts resolved.

cc: @jrbriggs, @derrickstolee

@mjcheetham
Copy link
Member Author

mjcheetham commented Oct 31, 2019

/azp run microsoft.scalar

Copy link
Member

@wilbaker wilbaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few questions after trying out the new code for a bit on the Mac.

Scripts/Mac/RunUnitTests.sh Show resolved Hide resolved

echo 'Running Scalar unit tests...'
$Scalar_PUBLISHDIR/Scalar.UnitTests || exit 1
dotnet publish $SCALAR_SRCDIR/Scalar.sln --runtime osx-x64 --configuration $CONFIGURATION || exit 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the move to .NET Core 3.0, should we still be able to build using VS for Mac? Or will we have to use the script?

I'm seeing this error when I try in the IDE:

image

Related, what is the best way to debug Scalar on Mac now? I used to be able to do the following:

  1. Build Scalar in VS for Mac
  2. Right-click on the project I want to run, and select Run With -> Custom Configuration ...
  3. Point VS at the Published binary I want to test
  4. Start debugging

But now that VS for Mac is unable to build, it does not appear to let me start debugging this way.

What do you use to debug .NET Core 3 applications on Mac?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you use to debug .NET Core 3 applications on Mac?

Related, how can we run the unit tests from an IDE on Mac?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related, how can we run the unit tests from an IDE on Mac?

To clarify, all I'm looking for is the best way to debug the unit tests on Mac. With the switch to using dotnet test I'm not sure what the best process is for doing so.

Copy link
Member Author

@mjcheetham mjcheetham Nov 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm seeing this error when I try in the IDE:

Ah.. this is great (and fixable).. So VS for Mac still uses Mono MSBuild to do it's 'in IDE' builds, not the dotnet CLI. The custom build tasks I have currently target the .NET Core RoslynCodeTaskFactory. I have an update that I'll push to fix this, and to select the correct factory depending on the MSBuild runtime.

image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related, what is the best way to debug Scalar on Mac now? I used to be able to do the following:

  1. Build Scalar in VS for Mac
  2. Right-click on the project I want to run, and select Run With -> Custom Configuration ...
  3. Point VS at the Published binary I want to test
  4. Start debugging

You do largely the same thing. If you just want to debug the project, right-click it in the Solution Pad and click ''Start Debugging Project":

image

To specify command-line arguments, use 'Run With' > 'Custom Configuration' again and pick 'Start Project' and 'Run Action: Debug' (and add your arguments or environment variables):

image

image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related, how can we run the unit tests from an IDE on Mac?

To clarify, all I'm looking for is the best way to debug the unit tests on Mac. With the switch to using dotnet test I'm not sure what the best process is for doing so.

Open the Unit Tests Pad under View > Pads > Unit Tests, select a test to run or debug!

image

image

image

You can also hit 'run all' and see all the lovely green ticks ✅ (hopefully)!
You will also see that the ignored tests are the Windows ones on Mac.

image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mjcheetham I tried grabbing the latest code from your branch (e3b2d34), and I'm now seeing a different build error in VS for Mac which prevents me from running the unit tests:

Build FAILED.

/Users/wilbaker/Repos/Scalar/src/Scalar.Common/Scalar.Common.csproj(24,29): error MSB4064: The "PackagePath" parameter is not supported by the "GetGitInstallerVersion" task. Verify the parameter exists on the task, and it is a settable public instance property.

/Users/wilbaker/Repos/Scalar/src/Scalar.Common/Scalar.Common.csproj(24,5): error MSB4063: The "GetGitInstallerVersion" task could not be initialized with its input parameters.
0 Warning(s)
2 Error(s)

Any ideas? I have tried nuking and building again but that did not seem to help.

I confirmed the following worked as expected on Mac:

  • Build Scalar from Terminal using BuildScalarForMac.sh
  • Run unit tests with RunUnitTests.sh (no warnings! 🎉 )
  • Run the install script that is placed in Scalar.Distribution.Mac

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case it matters, this is what dotnet --version reports:

~/Repos/Scalar/src/Scripts/Mac>dotnet --version
3.0.100

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mjcheetham as discussed offline, no need to block on VS for Mac support.

I was able to debug the unit tests using VSCode and the following approach:

  1. Open Terminal Window in VSCode
  2. export VSTEST_HOST_DEBUG=1
  3. dotnet test
  4. Attach to the PID reported in terminal:
Host debugging is enabled. Please attach debugger to testhost process to continue.
Process Id: 23870, Name: dotnet

If you know a better approach, please let me know 😄

Scalar.UnitTests/Program.cs Show resolved Hide resolved
Scalar.UnitTests/Program.cs Show resolved Hide resolved

set -e
. "$(dirname ${BASH_SOURCE[0]})/../InitializeEnvironment.sh"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this may need to be updated to:

"$(dirname ${BASH_SOURCE[0]})/InitializeEnvironment.sh"

When I ran this as it is now there was an error that InitializeEnvironment.sh could not be found:

~/Repos/Scalar/src/Scripts/Mac>./CreateScalarDistribution.sh 
./CreateScalarDistribution.sh: line 3: ./../InitializeEnvironment.sh: No such file or directory
No need to run this script anymore!
Run 'BuildScalarForMac.sh' and look in: /Scalar.Distribution.Mac/dist

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could also just remove this since it's just here to give a nice "error" to people with muscle memory. 😉

Copy link
Contributor

@derrickstolee derrickstolee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for merging #213 too early. I should have waited.

Here is a potential merge for you: 9827bdb

ECHO.
ECHO ==============================
ECHO Installing Git for Mac for Scalar
%SCALAR_DISTRIBUTION_ROOT%\Git\%GIT_INSTALLER_EXE% /DIR="C:\Program Files\Git" /NOICONS /COMPONENTS="ext,ext\shellhere,ext\guihere,assoc,assoc_sh" /GROUP="Git" /VERYSILENT /SUPPRESSMSGBOXES /NORESTART || EXIT /B 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This step is failing in the Windows functional tests. Should we change the "very silent" option until we know more about what's going on?

Copy link
Contributor

@derrickstolee derrickstolee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just making it clear that you can merge as soon as you have passing builds. Let's get this in and then see what small problems we have afterwards.

@derrickstolee derrickstolee merged commit 7b6f1cb into microsoft:master Nov 7, 2019
@derrickstolee
Copy link
Contributor

Thanks for your hard work here, @mjcheetham! I just merged to unblock the other PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants