Missing project files for `sample_project_fixes` test

[lextatic]

Added the project files for the sample_project_fixes tests.

But I think it won't be enough. It seems the command is being executed from the root of the repository, so it has to specify the workspace path where the project/sln file is located as the first CLI parameter when executing. Is something like this possible in the descriptor?

Maybe like this? (following what I saw in switft and repository descriptors)

    cli_lint_extra_args:
      - "{{WORKSPACE}}"
      - "--verify-no-changes"
      - "--exclude"
      - "/"
      - "--include"

[lextatic]

Everything should be okay now, the tests failing indicate that they're still expecting old dotnet 5.0 dotnet-format paremeters (with --check instead of --verify-no-changes).

Isn't there anywhere else that has to upgrade from dotnet 5.0 to dotnet 6.0?

[nvuillam]

@lextatic my bad, DevSkim and tsqllint use dotnet too... and I've not factorized such commands yet... I forgot to update their descriptors ! Now channel 6.0 is in all of them, I just pushed a new commit

Install commands are the following, are they still ok for dotnet v6 ?

      RUN wget --tries=5 -q -O dotnet-install.sh https://dot.net/v1/dotnet-install.sh \
          && chmod +x dotnet-install.sh \
          && ./dotnet-install.sh --install-dir /usr/share/dotnet -channel 6.0 -version latest
    - ENV PATH="${PATH}:/root/.dotnet/tools:/usr/share/dotnet"

[lextatic]

Install commands are the following, are they still ok for dotnet v6 ?

I wouldn't know, will have to look into it. I think we're still missing something.

Edit: I think it's alright -- https://docs.microsoft.com/en-us/dotnet/core/tools/dotnet-install-script

[lextatic]

@nvuillam So, I made some interesting discoveries. It seems dotnet-format is now part of the dotnet 6.0 sdk. With that we shouldn't need to run dotnet tool install -g dotnet-format anymore, and in fact when you install this it runs with an older version that isn't compatible with the --verify-no-changes argument.

Also, to use the dotnet format that comes with dotnet 6 we have to call dotnet format insead of dotnet-format (which executes the old, deprecated, dotnet-format tool). I'm not sure how to handle this change yet, looking into it.

[nvuillam]

So if I summarize well, we could do everything with just dotnet v6 install , and the only blocking stuff is DevSkim ? What if we removed DevSkim from general docker image and let it only in Security flavor ?

[lextatic]

So if I summarize well, we could do everything with just dotnet v6 install , and the only blocking stuff is DevSkim ? What if we removed DevSkim from general docker image and let it only in Security flavor ?

@nvuillam It seems tsqllint is having some trouble executing too. But unlike DevSkim I think this one is already upgradable to dotnet 6.0.

I think your suggested solution would do well, but if you really want we could make it work by installing both dotnet v5 and v6 and using pre and post commands to switch dotnet SDKs for DevSkim.

[nvuillam]

If you find a way to work with dotnet V5 and V6 it would indeed be the best ^^

[echoix]

If you find a way to work with dotnet V5 and V6 it would indeed be the best ^^

What I understand from using .NET, the idea of switching dotnet runtimes isn't really encountered, since the version selection just makes itself automatically without intervention. What we see is an error message when no compatible runtimes are installed for the app we want to run. It's really common on Windows to end up having each version/major version of dotnet installed, and there are no interventions to do to run a program. Just for fun, could you try installing both dotnet versions and see if it works without any extra configuration?

I know it's not ideal to have an extra runtime in the release image for nothing.

[nvuillam]

I think we tried that @lextatic & myself, but i'm not 100% sure, I never used dotnet so I have no clue about the solution, but let's try everything you think can be a good idea :)

[lextatic]

If you find a way to work with dotnet V5 and V6 it would indeed be the best ^^

What I understand from using .NET, the idea of switching dotnet runtimes isn't really encountered, since the version selection just makes itself automatically without intervention. What we see is an error message when no compatible runtimes are installed for the app we want to run. It's really common on Windows to end up having each version/major version of dotnet installed, and there are no interventions to do to run a program. Just for fun, could you try installing both dotnet versions and see if it works without any extra configuration?

I know it's not ideal to have an extra runtime in the release image for nothing.

@echoix It's true for dotnet runtime that several runtime versions can be installed and there's no "switching between runtimes". But dotnet-format is a SDK tool, not a runtime dotnet application and it will run using the whichever dotnet SDK you're using. On Windows, if you've got more than one SDK installed, it's usually the one being referenced in the %PATH environment variable. You can specify which SDK to run with a global.json file.

I was discussing with @nvuillam how to "switch sdks" in this thread, but it might not be necessary as DevSkim might upgrade soon.

[lextatic]

@nvuillam Well, after some changes now the linter is running smoothly, I'm positive it's linting both csharp and vb okay, but it's still failing on both test_failure and I have no clue how to proceed.

[github-actions[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

If you think this pull request should stay open, please remove the O: stale 🤖 label or comment on the pull request.

[nvuillam]

https://github.com/microsoft/DevSkim is still in alpha, as soon as they officially are released we'll try with their version working in v6 :)

[github-actions[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

If you think this pull request should stay open, please remove the O: stale 🤖 label or comment on the pull request.

[nvuillam]

@lextatic DevSkim latest version is not working with v6 (and not anymore with v5 ^^) I downgraded the DevSkim version for now, but it may be time to ressuscitate this PR :)

[lextatic]

@lextatic DevSkim latest version is not working with v6 (and not anymore with v5 ^^) I downgraded the DevSkim version for now, but it may be time to ressuscitate this PR :)

Okay! Let's do this. Sorry for the blackout, I was just waiting for the resolution on this one. So, what's the plan?

[nvuillam]

I think you can:

• make your branch up to date with main branch (with merge conflicts)
• remove the downgrade of DevSkim version in repository descriptor
• build again
• see what happens in CI :)

[lextatic]

I've updated it with the latest main and also changed PR name and target to main branch but I'm not completely sure how to proceed now with DevSkim and build.

[nvuillam]

@lextatic i took over you PR :p

[nvuillam]

@lextatic what could be wrong in this command ? ( it is supposed to not return 0 as there are formatting updates in files )

dotnet format /tmp/lint/.automation/test/csharp --verify-no-changes --exclude / --include /tmp/lint/.automation/test/csharp/csharp_bad_01.cs /tmp/lint/.automation/test/csharp/csharp_bad_02.cs | tee /dev/tty2 2>&1 && exit "${PIPESTATUS[0]}"

[lextatic]

@lextatic i took over you PR :p

No problem ;P

@lextatic what could be wrong in this command ? ( it is supposed to not return 0 as there are formatting updates in files )

dotnet format /tmp/lint/.automation/test/csharp --verify-no-changes --exclude / --include /tmp/lint/.automation/test/csharp/csharp_bad_01.cs /tmp/lint/.automation/test/csharp/csharp_bad_02.cs | tee /dev/tty2 2>&1 && exit "${PIPESTATUS[0]}"

Well, just dotnet format /tmp/lint/.automation/test/csharp --verify-no-changes --exclude / --include /tmp/lint/.automation/test/csharp/csharp_bad_01.cs /tmp/lint/.automation/test/csharp/csharp_bad_02.c is returning error code 2 here on Windows. Everything looks fine, I'll try to investigate it further.

[nvuillam]

This is still your PR, try new things with new commits if you like :)

[nvuillam]

Maybe an issue about absolute paths ?

[lextatic]

Maybe an issue about absolute paths ?

It looks like it's not finding the --include files. I'm trying to figure it out. Maybe adding a ./ for relative path on them?

Workspace path seems be fine though...

[lextatic]

It seems to be an issue with dotnet-format itself. I've posted an issue on their repo:

https://github.com/dotnet/format/issues/1770

Meanwhile I have some suggestions:

1. Since then dotnet 7.0 has already been officially released, what about upgrading it right to that instead of dotnet 6?

2. We could just remove the list-of-files option and ignore this error for now. I don't think that would be a big deal since the new dotnet format is going to analyze all files in the project anyway because it looks for dependencies and other things. What I mean is, list-of-files, or the --include option won't have any benefits in term of performance, and since it's not working properly we could just leave it out.

[lextatic]

@nvuillam Forgot to tag you in the previous comment, but I went on and decided to remove the list_of_files parameter for now and keep the dotnet6 since I wasn't able to make devskim and tsq-lint work with dotnet7. Please check the changes on DotnetFormatLinter.py to see if everything is okay there.

And there is still an issue with Terraform/Terrascan??? Can you check into that as well?

I had to sort the csharp and vbdotnet tests into separated good and bad folders in order to make it run properly, without the --include working I had to keep those files as distinct projects. I know it's not ideal, but we can revert this and put the --include paramenter back once I get an answer or fix from that issue I posted on the dotnet-format repo, but since the parameter is still not working even in the latest version of dotnet I thought it would be a better idea do leave it out.

[nvuillam]

@lextatic sorru for the delay... go for dotnet v7 if it has retrocompability with v6 :) ( I wouldn't like dotnet6 users to not be able to use ML )

[codecov-commenter]

Codecov Report

Merging #1680 (7f206ca) into main (1688acf) will increase coverage by 0.02%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1680      +/-   ##
==========================================
+ Coverage   82.97%   82.99%   +0.02%     
==========================================
  Files         170      170              
  Lines        4469     4470       +1     
==========================================
+ Hits         3708     3710       +2     
+ Misses        761      760       -1     

Impacted Files	Coverage Δ
megalinter/Linter.py	84.69% <100.00%> (+0.02%)	:arrow_up:
megalinter/linters/DotnetFormatLinter.py	100.00% <100.00%> (ø)
megalinter/utils.py	82.96% <100.00%> (ø)
megalinter/reporters/UpdatedSourcesReporter.py	89.74% <0.00%> (+2.56%)	:arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[bdovaz]

@lextatic note that the csharpier installation is set to 0.16.0 because in the main branch we are on .NET 5 but when you upgrade to .NET 7 you will probably have to remove "--version 0.16.0" to install the latest version.

[lextatic]

@lextatic note that the csharpier installation is set to 0.16.0 because in the main branch we are on .NET 5 but when you upgrade to .NET 7 you will probably have to remove "--version 0.16.0" to install the latest version.

Thanks, I'm removing the version arguments from the CLI but I think more stuff had failed because of the upgrade. Feel free to contribute with this PR if you know how to handle it.

If we can't do it we can revert it back to .NET 6 as it was working fine and passing all tests there. Keeping it as .NET 6 isn't that bad since .NET 6 is the LTS and .NET 7 is just a STS, meaning both will be supported until the release of .NET 8.