cyclonedx-dotnet
cyclonedx-dotnet copied to clipboard
CycloneDX
Enhancement request - do not assume 'obj' folder name
https://github.com/CycloneDX/cyclonedx-dotnet/blob/16ffa29cc371fbd99825b4afff644dd32fd53519/CycloneDX/Services/ProjectFileService.cs#L94
This code assumes the directory specified using the -biop flag is the parent of the obj folder. However, MSBuild provides you the ability to rename object folders to another name (such as .obj). Assuming the folder is always <parent>/obj/<subfolder> seems unnecessary. To avoid a breaking change here, you might be able to check if that path exists, and then use it, but otherwise try just directly using the directory specified with -biop.
To get around this, we are manually copying our intermediate directory to another location (<directory>/obj) and then passing <directory> to -biop flag.
Could you please clarify why you use a different name than ˋobjˋ? This is the default name for C#, C++, … and well known to developers.
| BaseIntermediateOutputPath | All | The top-level folder where all configuration-specific intermediate output folders are created. The default value is obj. The following code is an example: <BaseIntermediateOutputPath>c:\xyz\obj</BaseIntermediateOutputPath> |
|---|
see also :
- Setting BaseIntermediateOutputPath correctly in a SDK-based project is hard
- dotnet arcade: single build output
Could you please clarify why you use a different name than ˋobjˋ? This is the default name for C#, C++, … and well known to developers.
BaseIntermediateOutputPath All The top-level folder where all configuration-specific intermediate output folders are created. The default value is obj. The following code is an example: c:\xyz\obj</BaseIntermediateOutputPath> see also :
Why? I don’t suspect I have a good answer – it wasn’t even me that did it. But given that Microsoft lets you change this, it might be better to just relax the logic and require the entire path for CycloneDX. I don't see any gain from adding a magic folder named “obj”. It actually made it a little more annoying for us, because we have to figure out where the obj folder is, and then pass in the parent folder. :)
thanks, Matt
I think I actually saw code in another class that checks the proj file for the obj-output folder.
This shouldn't be a big deal, i'll try to push it in.
So i was able to reproduce it, by setting Project/PropertyGroup/BaseIntermediateOutputPath for example to intermediate\ then everything that usually goes into the obj-folder goes into intermediate.
Interestingly, even though there is a project property, setting it does not transfer everything, but only the content of obj\Debug. It must apparently be set in the Directory.Build.props.
My plan is to deprecate the current -biop / --BaseIntermediateOutputPath and add a new one, that doesn't assume the obj.
@mtsfoni Please double-check the usage of BaseIntermediateOutputPath and new artifacts properties introduced with .NET 8.0 "Simplified output path".
- UseArtifactsIntermediateOutput
- UseArtifactsOutput
- ArtifactsBinOutputName
- ArtifactsPackageOutputName
- ArtifactsPath
- ArtifactsPivots
- ArtifactsProjectName
- ArtifactsPublishOutputName
In my understanding this enhancement should be aligned with the .NET SDK approach and not possible MSBuild behavior.
C:\Program Files\dotnet\sdk\8.0.100\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.DefaultArtifactsPath.props
<!--
***********************************************************************************************
Microsoft.NET.DefaultArtifactsPath.props
WARNING: DO NOT MODIFY this file unless you are knowledgeable about MSBuild and have
created a backup copy. Incorrect changes to this file will make it
impossible to load or build your projects from the command-line or the IDE.
Copyright (c) .NET Foundation. All rights reserved.
***********************************************************************************************
-->
<Project>
<!-- This .props file may be imported either from Sdk.props or from Microsoft.NET.DefaultOutputPaths.targets, depending
on whether artifacts output properties were set in Directory.Build.props or not.
Set a property to indicate it was imported, so we can avoid a duplicate import. -->
<PropertyGroup>
<_DefaultArtifactsPathPropsImported>true</_DefaultArtifactsPathPropsImported>
</PropertyGroup>
<!-- Setting ArtifactsPath automatically opts in to the artifacts output format -->
<PropertyGroup Condition="'$(ArtifactsPath)' != '' And '$(UsingMicrosoftArtifactsSdk)' != 'true'">
<UseArtifactsOutput Condition="'$(UseArtifactsOutput)' == ''">true</UseArtifactsOutput>
<IncludeProjectNameInArtifactsPaths Condition="'$(IncludeProjectNameInArtifactsPaths)' == ''">true</IncludeProjectNameInArtifactsPaths>
<_ArtifactsPathLocationType>ExplicitlySpecified</_ArtifactsPathLocationType>
</PropertyGroup>
<!-- Set up base output folders if UseArtifactsOutput is set -->
<PropertyGroup Condition="'$(UseArtifactsOutput)' == 'true' And '$(ArtifactsPath)' == '' And '$(_DirectoryBuildPropsBasePath)' != ''">
<!-- Default ArtifactsPath to be in the directory where Directory.Build.props is found
Note that we do not append a backslash to the ArtifactsPath as we do with most paths, because it may be a global property passed in on the command-line which we can't easily change -->
<ArtifactsPath>$(_DirectoryBuildPropsBasePath)\artifacts</ArtifactsPath>
<IncludeProjectNameInArtifactsPaths Condition="'$(IncludeProjectNameInArtifactsPaths)' == ''">true</IncludeProjectNameInArtifactsPaths>
<_ArtifactsPathLocationType>DirectoryBuildPropsFolder</_ArtifactsPathLocationType>
</PropertyGroup>
<PropertyGroup Condition="'$(UseArtifactsOutput)' == 'true' And '$(ArtifactsPath)' == ''">
<!-- If there was no Directory.Build.props file, then put the artifacts path in the project folder -->
<ArtifactsPath>$(MSBuildProjectDirectory)\artifacts</ArtifactsPath>
<_ArtifactsPathLocationType>ProjectFolder</_ArtifactsPathLocationType>
</PropertyGroup>
</Project>
SDKs which are not shipped with .NET
- https://github.com/microsoft/MSBuildSdks/tree/main/src/Artifacts
- https://github.com/dotnet/arcade
@mtsfoni Please double-check the usage of BaseIntermediateOutputPath and new artifacts properties introduced with .NET 8.0 "Simplified output path".
- UseArtifactsIntermediateOutput
- UseArtifactsOutput
- ArtifactsBinOutputName
- ArtifactsPackageOutputName
- ArtifactsPath
- ArtifactsPivots
- ArtifactsProjectName
- ArtifactsPublishOutputName
In my understanding this enhancement should be aligned with the .NET SDK approach and not possible MSBuild behavior.
C:\Program Files\dotnet\sdk\8.0.100\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.DefaultArtifactsPath.props
<!-- *********************************************************************************************** Microsoft.NET.DefaultArtifactsPath.props WARNING: DO NOT MODIFY this file unless you are knowledgeable about MSBuild and have created a backup copy. Incorrect changes to this file will make it impossible to load or build your projects from the command-line or the IDE. Copyright (c) .NET Foundation. All rights reserved. *********************************************************************************************** --> <Project> <!-- This .props file may be imported either from Sdk.props or from Microsoft.NET.DefaultOutputPaths.targets, depending on whether artifacts output properties were set in Directory.Build.props or not. Set a property to indicate it was imported, so we can avoid a duplicate import. --> <PropertyGroup> <_DefaultArtifactsPathPropsImported>true</_DefaultArtifactsPathPropsImported> </PropertyGroup> <!-- Setting ArtifactsPath automatically opts in to the artifacts output format --> <PropertyGroup Condition="'$(ArtifactsPath)' != '' And '$(UsingMicrosoftArtifactsSdk)' != 'true'"> <UseArtifactsOutput Condition="'$(UseArtifactsOutput)' == ''">true</UseArtifactsOutput> <IncludeProjectNameInArtifactsPaths Condition="'$(IncludeProjectNameInArtifactsPaths)' == ''">true</IncludeProjectNameInArtifactsPaths> <_ArtifactsPathLocationType>ExplicitlySpecified</_ArtifactsPathLocationType> </PropertyGroup> <!-- Set up base output folders if UseArtifactsOutput is set --> <PropertyGroup Condition="'$(UseArtifactsOutput)' == 'true' And '$(ArtifactsPath)' == '' And '$(_DirectoryBuildPropsBasePath)' != ''"> <!-- Default ArtifactsPath to be in the directory where Directory.Build.props is found Note that we do not append a backslash to the ArtifactsPath as we do with most paths, because it may be a global property passed in on the command-line which we can't easily change --> <ArtifactsPath>$(_DirectoryBuildPropsBasePath)\artifacts</ArtifactsPath> <IncludeProjectNameInArtifactsPaths Condition="'$(IncludeProjectNameInArtifactsPaths)' == ''">true</IncludeProjectNameInArtifactsPaths> <_ArtifactsPathLocationType>DirectoryBuildPropsFolder</_ArtifactsPathLocationType> </PropertyGroup> <PropertyGroup Condition="'$(UseArtifactsOutput)' == 'true' And '$(ArtifactsPath)' == ''"> <!-- If there was no Directory.Build.props file, then put the artifacts path in the project folder --> <ArtifactsPath>$(MSBuildProjectDirectory)\artifacts</ArtifactsPath> <_ArtifactsPathLocationType>ProjectFolder</_ArtifactsPathLocationType> </PropertyGroup> </Project>SDKs which are not shipped with .NET
- https://github.com/microsoft/MSBuildSdks/tree/main/src/Artifacts
- https://github.com/dotnet/arcade
I would say that while this is true, my original point is still valid. I don't care what you set valid microsoft properties to - the code guessing that a folder named Obj should be used isn't valid if you are allowed to rename what the obj output folder is. I should be able to put those to D:\Cheese\ if I feel like it, and the tool should just allow me to specify that as a path. The proposed change sounds desirable, and I don't think there's anything that would need to be done to account for specific language constructs. Thoughts?
I think I'd prefer to stay stupid for now. The user can tell us where his assets file is, and that is from where we get it. Eventually we probably need to read the Directory.Builds.Probs, but this we can solve without.
What I am not sure about yet is the behaviour with --recursive. Actually, I am not even sure what happens now in the case that -biop and -rs are both set and what would actually make sense. (I think -rs is an annoying band aid to keep downwards compatibility for packages.config)
I agree this is not a pressing topic and not requested the last 4 years. Currently the value of MSBuild property BaseIntermediateOutputPath is not available and a possible solution is to introduce Buildalyzer ( see also #736). This will have an impact on performance but also provide access to the value of BaseIntermediateOutputPath.
@mtsfoni I never used the -rs option and think this is already obsolete. Typically legacy projects will not ask for SBOM ...
from C:\Program Files\dotnet\sdk\8.0.100\Sdks\Microsoft.NET.Sdk\Sdk\Sdk.props
<!-- If ArtifactsPath or UseArtifactsOutput are set, then import .props to set ArtifactsPath here, so that BaseIntermediateOutputPath can be
set in the ArtifactsPath.
If the .props file is not imported here, it will be imported from Microsoft.NET.DefaultOutputPaths.targets, so that artifacts output
properties can be set directly in the project file too (only in that case they won't affect the intermediate output). -->
<Import Project="$(MSBuildThisFileDirectory)..\targets\Microsoft.NET.DefaultArtifactsPath.props"
Condition="'$(UseArtifactsOutput)' == 'true' Or '$(ArtifactsPath)' != ''"/>
<PropertyGroup Condition="'$(UseArtifactsOutput)' == 'true'">
<UseArtifactsIntermediateOutput Condition="'$(UseArtifactsIntermediateOutput)' == ''">true</UseArtifactsIntermediateOutput>
<ArtifactsProjectName Condition="'$(ArtifactsProjectName)' == ''">$(MSBuildProjectName)</ArtifactsProjectName>
</PropertyGroup>
<PropertyGroup Condition="'$(UseArtifactsOutput)' == 'true' And '$(BaseIntermediateOutputPath)' == '' And '$(UseArtifactsIntermediateOutput)' == 'true'">
<BaseIntermediateOutputPath Condition="'$(IncludeProjectNameInArtifactsPaths)' == 'true'">$(ArtifactsPath)\obj\$(ArtifactsProjectName)\</BaseIntermediateOutputPath>
<BaseIntermediateOutputPath Condition="'$(BaseIntermediateOutputPath)' == ''">$(ArtifactsPath)\obj\</BaseIntermediateOutputPath>
</PropertyGroup>
from C:\Program Files\dotnet\sdk\8.0.100\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.DefaultOutputPaths.targets:
<!-- If "UseArtifactsOutput" wasn't set when the MSBuild project extensions .props files were imported, then use "obj" in the project folder for the intermediate output path
instead a folder under ArtifactsPath. To have the intermediate output path in the artifacts folder, "UseArtifactsOutput" should be set in Directory.Build.props-->
<PropertyGroup Condition="'$(UseArtifactsIntermediateOutput)' != 'true'">
<BaseIntermediateOutputPath Condition="'$(BaseIntermediateOutputPath)' == ''">obj\</BaseIntermediateOutputPath>
<BaseIntermediateOutputPath Condition="!HasTrailingSlash('$(BaseIntermediateOutputPath)')">$(BaseIntermediateOutputPath)\</BaseIntermediateOutputPath>
<IntermediateOutputPath Condition=" $(IntermediateOutputPath) == '' ">$(BaseIntermediateOutputPath)$(_PlatformToAppendToOutputPath)$(Configuration)\</IntermediateOutputPath>
<IntermediateOutputPath Condition="!HasTrailingSlash('$(IntermediateOutputPath)')">$(IntermediateOutputPath)\</IntermediateOutputPath>
</PropertyGroup>
@msherms2 what about if we just implement this one: Add support for single project.assets.json file In that case, instead of entering the path to the project.assets.json you could just enter the assets.json directly. If the project is properly setup (-rs not necessary, no packages.config-files, call .csproj not .sln) it should be able to get the same result as a call with a .csproj.
And I don't need to build fake projects for my functional tests that mostly run via asset files.
@msherms2 what about if we just implement this one: Add support for single project.assets.json file In that case, instead of entering the path to the project.assets.json you could just enter the assets.json directly. If the project is properly setup (-rs not necessary, no packages.config-files, call .csproj not .sln) it should be able to get the same result as a call with a .csproj.
And I don't need to build fake projects for my functional tests that mostly run via asset files.
I like the idea of a file instead of a object folder (I guess i now know why it needs it), but the project.assets.json files look per project, but I'm running Cyclone DX at the solution level and tracking my vulnerabilities from a component rather than project level. Would you be able to support wildcards or N files? I believe the current functionality takes one parent folder, appends obj to it (the request to change), and then does something like objFolder/**/project.assets.json. If you can do something like that in this design, I would have no problem with this approach. I didn't love the one object folder thing either but I didn't want to be too greedy.
The big problem with the call via .sln, or in this case a folder/wildcard, is always that there is not a real root-project. Every single project is just considered to be a part of the software. A few are filtered out, though (test-projects and dev-dependencies).
In my day-job, I have to do with many solutions that bundle both frontend and backend - basically two different projects, and we generate a sbom for each.
Anyway, if one would want to generate a sbom directly from multiple project.assets.json I have no information, what to put into the Metadata Component. I can read a project name and version from a single project.assets.json, but I have no way of knowing, which is the root-project (like the .exe or service). So that case could only work properly, if there is actually a metadata file imported.
The big problem with the call via .sln, or in this case a folder/wildcard, is always that there is not a real root-project. Every single project is just considered to be a part of the software. A few are filtered out, though (test-projects and dev-dependencies).
In my day-job, I have to do with many solutions that bundle both frontend and backend - basically two different projects, and we generate a sbom for each.
Anyway, if one would want to generate a sbom directly from multiple project.assets.json I have no information, what to put into the Metadata Component. I can read a project name and version from a single project.assets.json, but I have no way of knowing, which is the root-project (like the .exe or service). So that case could only work properly, if there is actually a metadata file imported.
I can understand your logic there, but I'm guessing that is a problem in my current use too, where i pass it one object folder. It's got to be iterating over all json files in there somehow and combining it. For help, here's the call I'm using, I think it's relatively self explanatory (even if it may not be fully correct :) )
dotnet CycloneDX "$slnFile" --disable-package-restore --base-intermediate-output-path $biopLoc --out $outDir --set-name $slnName --set-version $compVersion --set-type Library
I think in your proposed solution this would still work as --base-intermediate-output-path somehow becomes something that can either handle N json files or N object paths.
The hard coded obj folder in ProjectFileService.cs can be replaced with the property MSBuildProjectExtensionsPath (Buildalyzer will resolve the value).
This also allows to deprecate CycloneDX tool option "-biop, --base-intermediate-output-path".
GetProjectProperty could be replaced with GetMSBuildProjectExtensionsPath and the command line option -biop should be deprecated.
static internal string GetMSBuildProjectExtensionsPath(string projectFilePath)
{
var manager = new AnalyzerManager();
try
{
var project = manager.GetProject(projectFilePath);
var buildResults = project.Build();
return buildResults.Results.SelectMany(x => x.Properties).Where(x => x.Key.Equals("MSBuildProjectExtensionsPath")).First().Value;
}
catch (ArgumentException)
{
// can only happen while testing (because it will be checked before this method is called)
}
return "";
}
This might have gotten lost but again my only point is if you provide a lookup, just take the whole path. I wasn't trying to make this integrated with the MS project functionality whatsoever. I just want to be able to say -biop 'artifacts\myobjectfolder' if i want to dumbly name my obj folder that. Some code I can't control renames it to .obj, and while messy, cleaning it up because this code presumes to append /obj` is an easy problem to solve. You can even do it with backwards compatibility if you check if a 'obj' subfolder exists, use that instead, but otherwise use what is supplied.