Skip to content
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

support for dotnet based projects #1088

Conversation

durga-pasupuleti
Copy link

No description provided.

@prabhu
Copy link
Contributor

prabhu commented May 17, 2024

@dplabsindia, Thank you so much. Could you add some repotests as well?

@prabhu prabhu requested review from cerrussell and removed request for cerrussell May 17, 2024 00:52
index.js Outdated
@@ -4574,6 +4574,9 @@ export async function createCsharpBom(path, options) {
!paketLockFiles.length
) {
const filesToRestore = slnFiles.concat(csProjFiles);
console.error(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this message would always appear even before we restore?

index.js Outdated

if(dependsOn.name && dependsOn.version){
//console.log("prefixLLLL: ",prefix);
let dependcy = prefix+"/";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use template strings.

index.js Outdated
manifestFiles = manifestFiles.concat(csProjFiles);
// .csproj parsing
for (const f of csProjFiles) {
if (DEBUG_MODE) {
console.log(`Parsing ${f}`);
}
console.log(`csProjFiles List Size :: ${csProjFiles.length}`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These extra console.logs could be removed or wrapped behind if (DEBUG_MODE)

index.js Outdated
@@ -4771,7 +4816,8 @@ export async function createCsharpBom(path, options) {
}
if (FETCH_LICENSE) {
const retMap = await getNugetMetadata(pkgList, dependencies);
if (retMap.dependencies?.length) {
console.log(`dependencies re${retMap.dependencies?.length}`);
if (retMap.dependencies?.length > 1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why greater than 1?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an empty set for this. It adds nothing.

index.js Outdated
@@ -4884,11 +4930,13 @@ export function mergeDependencies(
provides: Array.from(provides_map[akey]).sort(),
});
} else {
if (deps_map && deps_map.size > 1){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why > 1 and not 0?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty set not needed for the sbom.

Copy link
Contributor

@prabhu prabhu May 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

greater than 1 means it will skip objects with just 1 entry.

@prabhu
Copy link
Contributor

prabhu commented May 17, 2024

  • Run npm run lint command to resolve biome issues
  • Needs unit tests and a repo tests for .Net framework

@prabhu
Copy link
Contributor

prabhu commented May 20, 2024

@durga-pasupuleti Are you planning to implement the PR comments, so that we can include this in 10.6.x?

@durga-pasupuleti
Copy link
Author

durga-pasupuleti commented May 20, 2024 via email

index.js Outdated
parentDependsOn.forEach((dependsOn) => {
if(dependsOn.name && dependsOn.version){
//console.log("prefix: ",prefix);
let dependcy = `${prefix}/`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the below lines can be combined to using template strings. This way we don't have to append strings together.

@@ -4772,10 +4769,11 @@ export async function createCsharpBom(path, options) {
if (dlist?.length) {
pkgList = pkgList.concat(dlist);
}

console.log(`pkgList List Size :: ${pkgList.length}`);
if(DEBUG_MODE){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please run npm run lint which will fix the linting issues automatically. Thank you so much for your patience.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran the lint. For some reason on my machine its producing error for directory name.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you share the error?

@setchy any ideas?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Os error 123, the filename , directory name or volume label syntax is incorrect

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a strange error. Could you contribute a test repo to add to the repotests? I will take care of the linting errors.

@prabhu
Copy link
Contributor

prabhu commented May 21, 2024

Could you kindly follow the instructions mentioned here to sign your commits?

https://github.com/CycloneDX/cdxgen/pull/1088/checks?check_run_id=25230419605

I think we are very close to getting this tested and merged!

index.js Outdated
ref: akey,
dependsOn: Array.from(deps_map[akey]).sort(),
});
if (deps_map && deps_map.size > 0){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a method called size? I usually do Object.keys(deps_map).length

}
if (parentDependsOn.size) {
const arrrr = new Set();
const prefix = parentComponent["bom-ref"].split("/")[0];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If prefix is just pkg:nuget we can directly used that in 4723.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason this without this its not able to create the dependencies. I checked multiple times for this.

index.js Outdated

if (dlist?.length) {
for (const p of dlist) {
parentDependsOn.add(p);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this not store the bom-ref similar to 4723. I think we don't need another loop in 4720.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, with these changes only i am able to generate sbom with dependencies.

Copy link
Contributor

@prabhu prabhu May 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second loop appears unnecessary. It is only checking if p has both name and version. Instead of storing p, you can store the string bom-ref from the second loop directly here

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes you are correct, Its a overlook. I just updated the code.

Durga Manikantha Pasupuleti added 5 commits May 21, 2024 20:16
Signed-off-by: Durga Manikantha Pasupuleti <durgamanikantha.pasupuleti@infor.com>
Signed-off-by: Durga Manikantha Pasupuleti <durgapasupuleti11@gmail.com>
Signed-off-by: Durga Manikantha Pasupuleti <durgamanikantha.pasupuleti@infor.com>
Signed-off-by: Durga Manikantha Pasupuleti <durgamanikantha.pasupuleti@infor.com>
Signed-off-by: Durga Manikantha Pasupuleti <durgamanikantha.pasupuleti@infor.com>
Signed-off-by: Durga Manikantha Pasupuleti <durgamanikantha.pasupuleti@infor.com>
Durga Manikantha Pasupuleti and others added 4 commits May 21, 2024 23:39
Signed-off-by: Durga Pasupuleti <durgapasupuleti11@gmail.com>
Signed-off-by: Durga Pasupuleti <durgapasupuleti11@gmail.com>
…sindia/cdxgen into feature/47frameworksupport

Signed-off-by: Durga Pasupuleti <durgapasupuleti11@gmail.com>
…ge. Signed-off-by: Durga Pasupuleti <durgapasupuleti11@gmail.com>
index.js Show resolved Hide resolved
@durga-pasupuleti
Copy link
Author

Could you kindly follow the instructions mentioned here to sign your commits?

https://github.com/CycloneDX/cdxgen/pull/1088/checks?check_run_id=25230419605

I think we are very close to getting this tested and merged!

I have added signoff by following the steps and added repotests. Anything i can do from my side.

@prabhu
Copy link
Contributor

prabhu commented May 23, 2024

@durga-pasupuleti, Thank you for this PR. It gave me the idea about what is missing. Let me collect some more examples and reimplement this slightly differently.

@prabhu
Copy link
Contributor

prabhu commented May 28, 2024

Superseded by #1119. Thank you so much for your guidance.

@prabhu prabhu closed this May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants