(GH-1020) Build with newer Mono

[TheCakeIsNaOH]

This fixes #1020

[jayvdb]

When I run this on openSUSE, I get one error

                                         [exec] Errors and Failures:
                                         [exec] 1) Test Error : chocolatey.tests.infrastructure.filesystem.DotNetFileSystemSpecs+when_finding_paths_to_executables_with_dotNetFileSystem_with_empty_path_extensions.GetExecutablePath_should_find_existing_executable
                                         [exec]    Should.Core.Exceptions.EqualException : Assert.Equal() Failure
                                         [exec] Position: First difference is at position 1
                                         [exec] Expected: /bin/ls
                                         [exec] Actual:   /usr/bin/ls
                                         [exec]   at (wrapper managed-to-native) System.Reflection.MonoMethod.InternalInvoke(System.Reflection.MonoMethod,object,object[],System.Exception&)
                                         [exec]   at System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x0006a] in <38fbd3d4e2894475b64d58b53bf42d47>:0

I thought it might be caused by a symlink /bin/ls -> /usr/bin/ls , but I deleted /bin/ls and it still failed.

My build is fixed with the following, but it likely needs to accept a list of possible values.

diff --git a/src/chocolatey.tests/infrastructure/filesystem/DotNetFileSystemSpecs.cs b/src/chocolatey.tests/infrastructure/filesystem/DotNetFileSystemSpecs.cs
index c4f067d9..03266d52 100644
--- a/src/chocolatey.tests/infrastructure/filesystem/DotNetFileSystemSpecs.cs
+++ b/src/chocolatey.tests/infrastructure/filesystem/DotNetFileSystemSpecs.cs
@@ -216,7 +216,7 @@ namespace chocolatey.tests.infrastructure.filesystem
             {
                 FileSystem.get_executable_path("ls").ShouldEqual(
                     Platform.get_platform() != PlatformType.Windows
-                        ? "/bin/ls"
+                        ? "/usr/bin/ls"
                         : "ls");
             }

[TheCakeIsNaOH]

It seems that Debian has ls in /bin, while OpenSuse has it in /usr/bin. I agree that the test should accept a list of values. I played around with it, and I just don't know enough c# to make the test accept both at the same time.

[jayvdb]

No worries; it is a separate problem, and only a test failure which doesnt effect the software.

[TheCakeIsNaOH]

Looking through issues, this should also close #369 and #1388.

[ferventcoder]

Formatting here looks a bit off.

And why add this when it already has the attribute of [WindowsOnly]?

[ferventcoder]

Perhaps the attribute needs adjusted?

[TheCakeIsNaOH]

The [WindowsOnly] attribute did not work for preventing the test from running on non-windows systems. I looked at the NUnit docs for the correct way to not run the test on non-windows, and it ended up being what I added, see link below.

https://github.com/nunit/docs/wiki/Platform-Attribute

Linux builds still work just fine without the [WindowsOnly] attribute, so I could remove it and leave only the [Platform(Exclude="")] if you think that would be best.

[TheCakeIsNaOH]

I went ahead and removed the [WindowsOnly] attribute.

[ferventcoder]

Sorry, what I was looking for was to fix the WindowsOnly attribute to ensure that it does prevent running on non-Windows systems. I don't want NUnit references required in here.

Reference using NUnit.Framework; at the top of this file.

[ferventcoder]

https://nunit.org/docs/2.6/platform.html This also highlights some things. I wonder if we could do an Include versus an exclude?

[ferventcoder]

Note it is specifically missing MacOSX, but that shouldn't affect Travis.

[ferventcoder]

So maybe we can flip to do an include on "Win" and see if that works?

[TheCakeIsNaOH]

public WindowsOnlyAttribute() : base("This is a Windows only test")

I tried adding this, it is still running the tests

If you want a go at it on windows, try changing the exclude to windows and build on windows then see if it skips the test.

So maybe we can flip to do an include on "Win" and see if that works?

Again, still running the tests on Linux.

[ferventcoder]

public WindowsOnlyAttribute() : base("This is a Windows only test")

I tried adding this, it is still running the tests

Yeah, that was for when it was ignore, we switched it to Platform attribute. Apologies.

So maybe we can flip to do an include on "Win" and see if that works?

Again, still running the tests on Linux.

Darn.

[ferventcoder]

Howdy @TheCakeIsNaOH - this is good - have a few minor thoughts and hoping to work through them with you. Any chance you are able to review them?

[TheCakeIsNaOH]

Hi, @ferventcoder. I replied/fixed the change requests.

Also, would you like me to update the how to install mono documentation in the readme in this PR, or should I open a new one?

[ferventcoder]

Update the readme on the mono docs

[steve-todorov]

@ferventcoder is here anything more needed for this PR to be merged? :)

[Cossey]

Would love for this to be merged so I can build some Linux docker containers from the official repo/current version.

There's still a request change but it seems like @TheCakeIsNaOH is waiting for a response from @ferventcoder, hopefully this can be merged soon; thanks Cake for this PR!

[carlspring]

We've been waiting for this fix for a while as well! @ferventcoder : Any updates, or ETA?

[steve-todorov]

Hey @ferventcoder, have you had a chance to check this PR again? We have a task which depends on this one and it's blocked for a while now. :)

[rekire]

Any updates here? This is a blocker for my build pipeline.

[carlspring]

Any update on this? It's been quite a while now...?

[Cossey]

@ferventcoder is everything all good now with these changes? Would like to start using most recent updates my Docker containers. Thanks!

[TheCakeIsNaOH]

@Cossey Right now the build on Linux is failing because tests are running when they are not supposed to. They were previously bypassed by a preprocessor if block, but mono no longer defines __MonoCS__ so the bypass is not working.

The correct way to run tests on specific platforms is via the platform attribute, which I added, and it worked correctly not running the test on Linux. However, @ferventcoder wants it abstracted into the TinySpec file, and provided an example. However, that example did not work, and have been unable to figure out how to get it working. See the open code review above.

[whitis]

Still breaks for original reason when cloned on ubuntu 18.04.2 LTS. It appears that even if you clone this modified repository instead of the main one, you don't get the changes.
sudo git clone https://github.com/TheCakeIsNaOH/choco.git
git branch --list

• master
  sudo ./build.sh
  BUILD FAILED

Mono 4.0 Profile (mono-4.0) is not installed, or not correctly configured.

The 'System.dll' assembly does not exist in framework assembly directory '/usr/lib/mono/4.0'.

For more information regarding the cause of the build failure, run the build again in debug mode.

Try 'nant -help' for more information

[TheCakeIsNaOH]

@whitis You need to switch to the MonoFixV2 branch, as I made the changes there instead of on master.

git checkout MonoFixV2

[gretel]

i'd love to see this merged. thank you!

[carlspring]

What's keeping this from being merged?

[steve-todorov]

Hi @ferventcoder, have you had a chance to check this yet? This has been blocking us for a while now. What is the current state? Are there any changes necessary? Could this be merged?

[jaalu]

Possibly mentioned upthread, but this is keeping at least one Docker image of Chocolatey on the Mono 3 image, which in turn is built on an old version of Debian (which is end-of-lifed IIRC, and lacks current certificates which breaks package pushing)

Having an "official" Linux-based Chocolatey container image would be a huge boon for CI pipelines and (mostly) reproducible building and packaging :)

Is the platform setup/TinySpec abstraction still the issue keeping this from being merged, or are there any other blockers that need to be solved?

[gretel]

any progress, please?

[steve-todorov]

+1 for @jaalu 's comment. We're also interested in building a choco image in alpine which will then be used for testing out our support for choco in Strongbox.

[ferventcoder]

I'm looking at merging this into stable, then bumping to master. Then we can look more closely into why the abstraction is not getting honored (I have a bad feeling something is doing something it shouldn't).

[gretel]

this PR has been submitted on 15 May 2019 and is still not merged.

[carlspring]

I agree!

@ferventcoder : What is the reason for this to take so long to be merged?

[nmaludy]

+1 on this, would love to see it merged!

[carlspring]

@ferventcoder : Any updates?

[steve-todorov]

@ferventcoder , @vexx32, @gep13 ping - is there any update on this PR? It has been here for almost a year and it's really making our tests for choco very painful (actually near impossible) in Jenkins. Would it be possible to have a release with this anytime within the next 6 months? :)

[carlspring]

...or even sooner? :)

[ferventcoder]

Rebased and merged to stable at c28a3e3. Merged to master at 254a42b.

Apologies on the slowness of getting this pulled in - appreciate that all involved have been patient on this. Happy building!

[carlspring]

Fantastic news! :)

[steve-todorov]

That's awesome!! Thanks so much for merging! Are you planning to make a 0.10.16 release soon?

[nmaludy]

WOOHOO!

[ferventcoder]

@steve-todorov we won't need an actual release of Choco for you to take advantage of this. You can build it now.