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

Path Excludes not working when workspaces are introduced in the code repository based on NPM, PNPM, Yarn2 #8940

Closed
porsche-rishisaxena opened this issue Jul 26, 2024 · 4 comments · Fixed by #9247
Labels
analyzer About the analyzer tool bug Issues that are considered to be bugs

Comments

@porsche-rishisaxena
Copy link

porsche-rishisaxena commented Jul 26, 2024

Describe the bug

Path Excludes not working when workspaces are introduced in the code repository based on NPM, PNPM, Yarn2.

To Reproduce

Steps to reproduce the behavior:

  1. Sample Code Repo created to reproduce the behavior: https://github.com/porsche-ozgursedefoglu/npm-workspaces
  2. skip_excluded: true configured in .ort.yml. Please refer to https://github.com/porsche-ozgursedefoglu/npm-workspaces/blob/main/.ort.yml
  3. Run the analyzer on the above repo
    docker run -v $PWD:/project
    ort:latest
    -P ort.analyzer.allowDynamicVersions=true --debug analyze
    -i /project/
    -o /project/ort/

Expected behavior

After Path Exclude applied in .ort.yml, the paths are still visible in the reports which should not be the case and the below statement is violated:
"ORT can be configured to skip them completely during the analysis phase. The affected elements are then not processed any further and do not occur in generated reports."

@porsche-rishisaxena porsche-rishisaxena added bug Issues that are considered to be bugs to triage Issues that need triaging labels Jul 26, 2024
@fviernau
Copy link
Member

"ORT can be configured to skip them completely during the analysis phase. The affected elements are then not processed any further and do not occur in generated reports."

@porsche-rishisaxena have you configured ORT to do so? (If yes, this could be added to the reproduce step after #1.)

@porsche-ozgursedefoglu
Copy link

@fviernau we did this step in our .ort.yml file which you can see in the sample code repository as following;
analyzer:
skip_excluded: true

@porsche-rishisaxena
Copy link
Author

@fviernau I have added the second step in the problem statement description what @porsche-ozgursedefoglu mentioned where we use skip_excluded: true. Please refer to the sample repo we created for reproducing: https://github.com/porsche-ozgursedefoglu/npm-workspaces/blob/main/.ort.yml

@sschuberth sschuberth added analyzer About the analyzer tool and removed to triage Issues that need triaging labels Aug 13, 2024
fviernau added a commit that referenced this issue Oct 7, 2024
Previously, the submodules of any `Yarn` or `Pnpm` workspaces were
represented as packages. This was inconsistent, because ORT normally
represents any definition files found in the analyzed sources as a
`Project`, not as a `Package`. Besides being inconsistent, the `Package`
representation renders the `ort.yml` features unusable. For example,
workspace submodules could not be excluded via path excludes.
Furthermore, the previous implementation represented the workspace root
project (in case of Pnpm) as both, as a `Package` and as a `Project`.
Finally, the previous Package representation of a submodule did not have
any reference from any project scope. As a consequnce, any (license)
policy rules which operates only on non-excluded dependencies would have
disregarded the submodules and their transitive dependencies,
potentially leading to an incorrect underreporting of rule violations.

Extend the `NpmModuleInfo` class by the flag `isProject` and make use of
it in the `NpmDependencyHandler` for creating the packages and
determining the linkage type. This change guarantees that
`parsePackage()` is no more called for `Project`s, but only for
`Package`s which is why the project-specific logic is dropped from
`parsePackage()`. For projects the dedicated `parseProjects()` is now
consistently used instead.

As in the new representation there are no more unreferenced packages,
the dependency handler does take care of creating all packages. So, the
logic which calls `graph.addPackage()` for each module returned by
`parseInstalledModules()` became unnecessary and is dropped.

Note: This commit fixes multiple things at once, because it seemed too
complicated to fix each issue separately due to various chicken-egg
like problems.

Fixes #9196, fixes #8940.

Signed-off-by: Frank Viernau <[email protected]>
fviernau added a commit that referenced this issue Oct 8, 2024
Previously, the submodules of any `Yarn` or `Pnpm` workspaces were
represented as packages. This was inconsistent, because ORT normally
represents any definition files found in the analyzed sources as a
`Project`, not as a `Package`. Besides being inconsistent, the `Package`
representation renders the `ort.yml` features unusable. For example,
workspace submodules could not be excluded via path excludes.
Furthermore, the previous implementation represented the workspace root
project (in case of Pnpm) as both, as a `Package` and as a `Project`.
Finally, the previous Package representation of a submodule did not have
any reference from any project scope. As a consequnce, any (license)
policy rules which operates only on non-excluded dependencies would have
disregarded the submodules and their transitive dependencies,
potentially leading to an incorrect underreporting of rule violations.

Extend the `NpmModuleInfo` class by the flag `isProject` and make use of
it in the `NpmDependencyHandler` for creating the packages and
determining the linkage type. This change guarantees that
`parsePackage()` is no more called for `Project`s, but only for
`Package`s which is why the project-specific logic is dropped from
`parsePackage()`. For projects the dedicated `parseProjects()` is now
consistently used instead.

As in the new representation there are no more unreferenced packages,
the dependency handler does take care of creating all packages. So, the
logic which calls `graph.addPackage()` for each module returned by
`parseInstalledModules()` became unnecessary and is dropped.

Note: This commit fixes multiple things at once, because it seemed too
complicated to fix each issue separately due to various chicken-egg
like problems.

Fixes #9196, fixes #8940.

Signed-off-by: Frank Viernau <[email protected]>
fviernau added a commit that referenced this issue Oct 8, 2024
Previously, the submodules of any `Yarn` or `Pnpm` workspaces were
represented as packages. This was inconsistent, because ORT normally
represents any definition files found in the analyzed sources as a
`Project`, not as a `Package`. Besides being inconsistent, the `Package`
representation renders the `ort.yml` features unusable. For example,
workspace submodules could not be excluded via path excludes.
Furthermore, the previous implementation represented the workspace root
project (in case of Pnpm) as both, as a `Package` and as a `Project`.
Finally, the previous Package representation of a submodule did not have
any reference from any project scope. As a consequnce, any (license)
policy rules which operates only on non-excluded dependencies would have
disregarded the submodules and their transitive dependencies,
potentially leading to an incorrect underreporting of rule violations.

Extend the `NpmModuleInfo` class by the flag `isProject` and make use of
it in the `NpmDependencyHandler` for creating the packages and
determining the linkage type. This change guarantees that
`parsePackage()` is no more called for `Project`s, but only for
`Package`s which is why the project-specific logic is dropped from
`parsePackage()`. For projects the dedicated `parseProjects()` is now
consistently used instead.

As in the new representation there are no more unreferenced packages,
the dependency handler does take care of creating all packages. So, the
logic which calls `graph.addPackage()` for each module returned by
`parseInstalledModules()` became unnecessary and is dropped.

Note: This commit fixes multiple things at once, because it seemed too
complicated to fix each issue separately due to various chicken-egg
like problems.

Fixes #9196, fixes #8940.

Signed-off-by: Frank Viernau <[email protected]>
sschuberth pushed a commit that referenced this issue Oct 8, 2024
Previously, the submodules of any `Yarn` or `Pnpm` workspaces were
represented as packages. This was inconsistent, because ORT normally
represents any definition files found in the analyzed sources as a
`Project`, not as a `Package`. Besides being inconsistent, the `Package`
representation renders the `ort.yml` features unusable. For example,
workspace submodules could not be excluded via path excludes.
Furthermore, the previous implementation represented the workspace root
project (in case of Pnpm) as both, as a `Package` and as a `Project`.
Finally, the previous Package representation of a submodule did not have
any reference from any project scope. As a consequnce, any (license)
policy rules which operates only on non-excluded dependencies would have
disregarded the submodules and their transitive dependencies,
potentially leading to an incorrect underreporting of rule violations.

Extend the `NpmModuleInfo` class by the flag `isProject` and make use of
it in the `NpmDependencyHandler` for creating the packages and
determining the linkage type. This change guarantees that
`parsePackage()` is no more called for `Project`s, but only for
`Package`s which is why the project-specific logic is dropped from
`parsePackage()`. For projects the dedicated `parseProjects()` is now
consistently used instead.

As in the new representation there are no more unreferenced packages,
the dependency handler does take care of creating all packages. So, the
logic which calls `graph.addPackage()` for each module returned by
`parseInstalledModules()` became unnecessary and is dropped.

Note: This commit fixes multiple things at once, because it seemed too
complicated to fix each issue separately due to various chicken-egg
like problems.

Fixes #9196, fixes #8940.

Signed-off-by: Frank Viernau <[email protected]>
fviernau added a commit that referenced this issue Oct 8, 2024
Previously, the submodules of any `Yarn` or `Pnpm` workspaces were
represented as packages. This was inconsistent, because ORT normally
represents any definition files found in the analyzed sources as a
`Project`, not as a `Package`. Besides being inconsistent, the `Package`
representation renders the `ort.yml` features unusable. For example,
workspace submodules could not be excluded via path excludes.
Furthermore, the previous implementation represented the workspace root
project (in case of Pnpm) as both, as a `Package` and as a `Project`.
Finally, the previous Package representation of a submodule did not have
any reference from any project scope. As a consequnce, any (license)
policy rules which operates only on non-excluded dependencies would have
disregarded the submodules and their transitive dependencies,
potentially leading to an incorrect underreporting of rule violations.

Extend the `NpmModuleInfo` class by the flag `isProject` and make use of
it in the `NpmDependencyHandler` for creating the packages and
determining the linkage type. This change guarantees that
`parsePackage()` is no more called for `Project`s, but only for
`Package`s which is why the project-specific logic is dropped from
`parsePackage()`. For projects the dedicated `parseProjects()` is now
consistently used instead.

As in the new representation there are no more unreferenced packages,
the dependency handler does take care of creating all packages. So, the
logic which calls `graph.addPackage()` for each module returned by
`parseInstalledModules()` became unnecessary and is dropped.

Note: This commit fixes multiple things at once, because it seemed too
complicated to fix each issue separately due to various chicken-egg
like problems.

Fixes #9196, fixes #8940.

Signed-off-by: Frank Viernau <[email protected]>
@sschuberth
Copy link
Member

@porsche-rishisaxena it would be great if you could confirm on your end that #9247 fixes your issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer About the analyzer tool bug Issues that are considered to be bugs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants