-
Notifications
You must be signed in to change notification settings - Fork 17
feat: add support for .net recognition (#51) #52
Conversation
Signed-off-by: Luca Stocchi <lstocchi@redhat.com>
Signed-off-by: Luca Stocchi <lstocchi@redhat.com>
Signed-off-by: Luca Stocchi <lstocchi@redhat.com>
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.
See comments inline.
Also a question: it this a breaking change for the API point of view. Because the latest change causes a lot of issues for JBoss Tools
java/alizer-api/src/main/java/com/redhat/devtools/alizer/api/ComponentRecognizerImpl.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Luca Stocchi <lstocchi@redhat.com>
Hey folks! I have some thoughts and suggestions on this. For some background, the ".NET" umbrella includes a bunch of different things, and it's easy to mix them up. I am still not sure I understand all the moving parts myself, especially for older (proprietary, not cross platform) .NET.
You can probably guess the language from the file extension. There's only one build system: msbuild. Project files use a bit of a flexible extension system. They pretty much always end with
The Runtime is specified in the project file with things like The older runtime versions (.NET Framework 3.x and 4.x) are proprietary and Windows only. They are not supported on Linux. If you wanted to build/run this on OpenShift, you would need to run Windows, somehow. The cross-platform versions are .NET Core (all versions) with the The project type is a little tricky. It might be listed in the project file as a simple dependency, but the most common framework for recent .NET projects (ASP.NET Core) is implied by |
C#: | ||
configuration_files: | ||
- ".csproj" | ||
- "appsettings.json" |
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.
This file could be named appsettings.${SOMETHING}.json
as well, where ${SOMETHING}
could be pretty much anything, with Development
and Production
being common values.
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.
So my question is ... is this file really useful for us? From your explanation it looks to me we would be good by only using .(cs/fs/vb/something)proj
to detect frameworks (.netcore, .net5/6), right?
The other thing would be to find out which are the project dependencies but they should be stored into packages.config which is read by nuget, no? So what appsettings is usually used for? Could it contain some infos useful for us or can we avoid checking for it?
The same for app.config
and Web.config
. I added them to the list because when i was looking at some vb.net project online i saw they contained some infos but maybe they are not necessary anymore if we target .net 5/6/core?
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.
My understanding is that appsettings*.json
files only contain application-specific runtime configuration. Some common examples are logging configuration and database connection strings (which provide a good hint to which database, if any, is being used). They aren't used at build time by the SDK.
Project dependencies are generally stored in the project file (.csproj
, etc) with modern .NET (.NET Core, .NET >= 5). The build system reads it from there and uses nuget under the hood to download them.
AFAIK, app.config
and web.config
are older .NET files. They are used by classic ASP.NET which I am not familiar with at all. If alizer wants to support only modern .NET, it probably doesn't need to look at them.
resources/projects/VB.NET-ECommerce/App_Data/PublishProfiles/cis466shoestore.pubxml
Outdated
Show resolved
Hide resolved
Signed-off-by: Luca Stocchi <lstocchi@redhat.com>
After sleeping on this, a few other things to watch out for.: It's possible to have a single project that target multiple runtimes. The SDK will build your source code multiple times, each time targeting a different runtime. This generally involves using a semicolon-delimeted list in the Project files can contain embedded snippets of code, such as Property Functions. A very common use looks like this: <Import Project="$([MSBuild]::GetPathOfFileAbove(dir.props))" Condition=" '$([MSBuild]::GetPathOfFileAbove(dir.props))' != '' " /> Which uses And that brings me to If you really want to fully parse project files, I am not sure you can do that without either calling msbuild ( |
Signed-off-by: Luca Stocchi <lstocchi@redhat.com>
Signed-off-by: Luca Stocchi <lstocchi@redhat.com>
@jeffmaury i updated the PR and I added almost everything suggested by @omajid but the Property functions. This requires a deeper investigation and i think this is not required for a first release and for the current state of .net devfiles. I'm going to opena Thanks @omajid for all your useful infos, really appreciated!! 🙏 I'm going to look at your comments again and again in future to improve the detection. |
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.
LGTM
it resolves #51
I changed the way to detect a devfile. Now it first uses component detection and then, if nothing is found, language detection.
This way i give priority to languages with frameworks and it seems the results are more stable.
P.S: Many of those 213 changed files are projects (c#/vb.net) for testing.