Linux support with additional test project

[garyhuntddn]

Hi - I hit the issue with not being able to use WebCompiler in a docker build. So thought I'd have a go at tackling *nix support.

I've had to create an additional test project to be able to run the tests on OS X as the original test project targets .Net v4.8 - I've kept the artefacts in their original location though to reduce the duplication.

The highest risk change I've made is to FileHelpers.MakeRelative - this wasn't interested in working at all on OS X so I've started from scratch. I've added a few tests but there are a number of scenarios that it doesn't currently deal with such as the case-senstive/insensitive nature of *nix vs Windows.

For now I've implemented the code in place - but I would anticipate pulling the repetitive code out into a helper function.

Nearly all of tests are running under OS X - but it's a start - the ones that are failing are having issues where the LastWriteTimeUtc is being set. I need to compare a Windows and an OS X run to see if that's simply a limitation in *nix.

Have a look and see what you think.

[nukithelegend]

@failwyn is this repo still active and maintained?

[garyhuntddn]

There were updates a few months ago - so I've assumed it is - @failwyn is certainly replying to things, so I'm happy to put some effort in to get it to a useful place for us docker users

[failwyn]

@failwyn is this repo still active and maintained?

Yes, it’s still active and maintained; I just don’t currently have a way to test Unix or Mac.

[nukithelegend]

@failwyn is this repo still active and maintained?

Yes, it’s still active and maintained; I just don’t currently have a way to test Unix or Mac.

Can you maybe consider setting up a Unix VM on your Windows box to review the PR?

[failwyn]

@nukithelegend I'll need to find time to review all the code in the pull request, if the code is clean and stable on Windows amd64 and arm64, I'll look into getting something set up to test *nix platforms.

[garyhuntddn]

If you want me to setup an AWS VM for a month or so then thats easily done

[failwyn]

@garyhuntddn I appreciate the offer, but I intend on maintaining this until Visual Studio implements native support, so I need to plan for a permanent solution.

[Velociraptor45]

Hey, is there anything we can do to help get this done? Linux support would be really needed on my end here for building Docker images.

[garyhuntddn]

Hey, is there anything we can do to help get this done? Linux support would be really needed on my end here for building Docker images.

Hi Velociraptor45 - there's a few bits that need expanding upon and some edge cases we should deal with.

This is a good opportunity for me to write down where I've got to so that I/we can consider the next steps prior to failwyn taking over and accepting any form of pull-request.

1. Building the nuget package - the current solution that builds under windows (and now that I know the codebase a bit better I realise there's no need to support building under *nix) includes 7z, and a bunch of npm packages that are bundled up into 7z files. Then during your own solution compile these files are un-7z'ped out to a temp folder and used to compile things. Yep - it includes a complete nodejs environment bundled in the nuget.

We have a few options here:

a) stick with the existing bundle (note 2 below) and rely upon having nodejs and 7z as *nix runtime dependencies (during your own solution build) that need to be installed and in the users path - the Windows solution bundles them which is fair enough as it's less likely these will be installed on your average VS developers machine b) we could also bundle them in an updated package that includes both Windows and *nix dependencies. My Personal preference is to avoid that as the nuget is far larger than it truly needs to be anyway. *nix also comes in more flavours and I'd want to support x86, amd64 and arm64 c) persuade failwyn to switch to a .tar.gz format which would remove the 7z linux dependency - I'm not a fan of this as we still have a nodejs dependency, all we're getting rid of is the *nix 7z dependency (Windows will still use 7z to extract the .tzr.gz) d) update the codebase to install their own nodejs and npm packages from official sources when required and not zip them up as a dependency of the nuget. I like this solution as it would be more future proof, but this feels like too much change without enough benefit.

My preference here is a) as it requires least effort from all involved.

2. There was an issue installing the nodejs packages on *nix due to a package being deprecated. I've a feeling that failwyn has actually dealt with this one and it was only being kept for backward-compatibility

3. There's a few unit tests failing.

4. It would be good to have some *nix unit tests - but that would require building on a non-Windows machine and wouldn't fit failwyn's dev. environment and I fully respect the extra effort required with very little gain. My fork also includes a revised version of the test project built upon .Net core to allow me to run it on Windows and *nix. There's a good argument to switch the core project across to this version - but again - entirely up to failwyn as it's his time and effort we might be consuming. For now the tests have done their job in allowing me to understand the codebase and to get it working.

5. Testing on docker as that too is my primary use case. This will require updated documentation to state the steps of installing both nodejs and 7z into the docker build environment (you're always going to need nodejs if you're going to be doing sass/typescript transpiling during the docker build so not too onerous) and a bunch of us then offer our time and effort to stick with the project and assist where needed.

From my perspective there are a number of goals:

a) as few changes as possible b) don't break the Windows build as that is the 90-95% use case c) support the use in Docker as I can see that forming a 5-10% use case d) nice-to-have - support building and testing on any platform as it might support build automation in the future

[Velociraptor45]

I now really tried to get this solution to compile - but it won't. It does not seem to recognize my .net framework 4.8 (+ targeting-package) installation and is missing some node_modules folder. Do you have any "How to build" instructions?

[Velociraptor45]

I finally got the project itself to run, but there seem to be a couple of implementations in the FileHelpers.cs that are not supported in .net framework. Also, the compiler initialization has a huge performance issue due to the node_exe variable missing the full path.

If you like, garyhuntddn, I'd fork your repo, fix this stuff & the tests and think about how we might get this into Docker. But in general, I'd also go for your 1a) suggestion and get this to *nix at all in a first step and then maybe later on refactor it in a different PR.

[failwyn]

I agree with the approach of doing 1a then the refactor as a separate PR.

[Velociraptor45]

I've put a couple of hours now into this, but can't get some unit tests to work. This seems to be already an issue on your master branch, @failwyn. When executing the test ScssTest.CompileScss, the second entry in the scssconfig.json runs in the ConfigFileProcessor in an early exit (and thus skips the relative path adjustment) because the compiler result contains an error. Are you aware of that?

node:internal/modules/cjs/loader:933
   const err = new Error(message);
               ^
 
 Error: Cannot find module '../doc/directives.js'
 Require stack:
 - C:\Users\runneradmin\AppData\Local\Temp\WebCompiler1.14.10\node_modules\yaml\dist\compose\composer.js
 - C:\Users\runneradmin\AppData\Local\Temp\WebCompiler1.14.10\node_modules\yaml\dist\index.js
 - C:\Users\runneradmin\AppData\Local\Temp\WebCompiler1.14.10\node_modules\postcss-load-config\src\index.js
     at Function.Module._resolveFilename (node:internal/modules/cjs/loader:933:15)
     at Function.Module._load (node:internal/modules/cjs/loader:778:27)
     at Module.require (node:internal/modules/cjs/loader:1005:19)
     at require (node:internal/modules/cjs/helpers:94:18)
     at Object.<anonymous> (C:\Users\runneradmin\AppData\Local\Temp\WebCompiler1.14.10\node_modules\yaml\dist\compose\composer.js:3:18)
     at Module._compile (node:internal/modules/cjs/loader:1101:14)
     at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
     at Module.load (node:internal/modules/cjs/loader:981:32)
     at Function.Module._load (node:internal/modules/cjs/loader:822:12)
     at Module.require (node:internal/modules/cjs/loader:1005:19) {
   code: 'MODULE_NOT_FOUND',
   requireStack: [
     'C:\\Users\\runneradmin\\AppData\\Local\\Temp\\WebCompiler1.14.10\\node_modules\\yaml\\dist\\compose\\composer.js',
     'C:\\Users\\runneradmin\\AppData\\Local\\Temp\\WebCompiler1.14.10\\node_modules\\yaml\\dist\\index.js',
     'C:\\Users\\runneradmin\\AppData\\Local\\Temp\\WebCompiler1.14.10\\node_modules\\postcss-load-config\\src\\index.js'
   ]
 }

I can reproduce the behavior locally and in a build pipeline that I'm currently setting up.

[failwyn]

@Velociraptor45 If you can add that as a new issue, I’ll take a look at it. Thanks for the heads up!

[Velociraptor45]

Ok, I think we're almost there. I just opened a PR to garyhuntddn's branch where I fixed the remaining issues (esp. directory separator & file capitalization for Linux) & created pipelines for .net Core on Ubuntu, .net framework on Windows & .net Core on Windows. Sadly, I don't own any machines running macOS, thus I couldn't test that. However, for the time being, I commented out the failing SCSS tests to get the new pipelines green.