-
Notifications
You must be signed in to change notification settings - Fork 14
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
Upgrade to dotnet core 2.2 #73
Conversation
<include>*.exe</include> | ||
<include>*.config</include> | ||
<include>*/FSharp.Core.resources.dll</include> | ||
<include>win-x86/**</include> |
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.
Plugin is now quiet large - about 100MB. That's because of 3 self-contained dotnet runners.
On out server the next largest is Cfamily-plugin with only 27MB.
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.
Yes, this was my concern! most likely this will be a problem if uploading the plugin to marketplace. 100MB might also affect analysis times.
the alternative as I mentioned, would be to force users to have a dot net core runtime. What is your opinion?
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.
also could be considerer to be create as a dotnet global tool.
Ex: dotnet tool install --global --add-source . OpenCoverPathConverter => https://github.com/jmecsoftware/OpenCover2SonarConverter/releases
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.
Either require users to have dotnet core installed or have different budles depending on OS.
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.
Now configured it as Framework-dependent executables (FDE). This includes all except the dotnet core runtime.
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.
jar file is about 20MB for all 3 operating systems
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.
reverted
This comment has been minimized.
This comment has been minimized.
b2fdd08
to
ff69c5e
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
[22:25:12] Seems the FsSonarRunner is being packaged without execution permissions. |
[22:30:57] |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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.
* Did you use the build from appveyor on your machines for sonar execution or your local build? * In the log I can see the three dotnet-publish steps. Are the results available in `FsSonarRunner\FsSonarRunner\publish? In the log is unfortunately only Maven output, not the dotnet Output to console included.
yep, all 3 targets were created. But still it did fail
<include>*.exe</include> | ||
<include>*.config</include> | ||
<include>*/FSharp.Core.resources.dll</include> | ||
<include>win-x86/**</include> |
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.
Yes, this was my concern! most likely this will be a problem if uploading the plugin to marketplace. 100MB might also affect analysis times.
the alternative as I mentioned, would be to force users to have a dot net core runtime. What is your opinion?
<include>*.exe</include> | ||
<include>*.config</include> | ||
<include>*/FSharp.Core.resources.dll</include> | ||
<include>win-x86/**</include> |
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.
also could be considerer to be create as a dotnet global tool.
Ex: dotnet tool install --global --add-source . OpenCoverPathConverter => https://github.com/jmecsoftware/OpenCover2SonarConverter/releases
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Wouldn't "to be create as a dotnet global tool" also require an Installation on the build server of dotnet Core? I'm reluctant as I just failed with FDE deployment - where only .NET Core 2.1 was installed. |
Tried to set executable flag from Java. Can you please test if this works? |
yeah your are right! so lets get the flags working |
works now, however the hack https://github.com/jmecsoftware/sonar-fsharp-plugin/blob/8025038c0431444e4311eca378f0da512e4b6da7/FsSonarRunner/FsSonarRunner/Program.fs#L70 that ive made now is cutting one extra /, so it can find any file. Do u know how to pass absolute path from java side directly? the hack was done originally because i was not able to find a way to pass absolute path from java side since they change quite a bit the api. |
# Conflicts: # README.md # appveyor.yml # sonar-communityfsharp-plugin/src/main/java/org/sonar/plugins/fsharp/FSharpSensor.java # sonar-communityfsharp-plugin/src/main/java/org/sonar/plugins/fsharp/utils/OSInfo.java
SonarQube gves the hint, it might be not an file URI. But otherwise, a not so elegant solution would be
Update: As this is Independent of this PR I'll make a small separate PR #77 for this. |
comment on PR #73 to URI hack
i still do a quick test, see if everything works in linuyx and osx |
Linux ok OSx nok, Supose we could for now just release windows and linux suport, leave OSx for another time |
I'll go and evaluate if changing permissions was successful. For me it's ok to remove OS-X, the jar will get smaller. But we should then do a quick retest on the systems again. |
linux ok |
@jmecosta Did you close this PR by intend without merging? |
As proposed I removed Thursday evening OS-X from the build (which shrunk the jar). As I never ever worked with Apple products: Can we build self-contained .NET Core for OS-X on Windows? As far as I remember it was not possible for Xamarin. |
Not sure what happen, was supose to be an approve |
Seems like current build 248 is running for me on windows. If you can approve it for Linux, it's good. On OS-X the build should still work. I saw there is some possibility to add an warning when an analysis was not executed, maybe we can include something like that in the furture. Personally I would like to merge this first. |
Go ahead then. All ok for me |
see discussion in Issue #26
FsSonarRunner upgraded to self-contained dotnet Core 2.2 application. Build contains runners for
Os-X x 64As I have only Windows build servers would be good if someone could also test Linux
or Os-Xenvironment.Update: OX-X canceled after first Tests and to shrink jar.