Skip to content

Conversation

figueroa1395
Copy link
Member

@figueroa1395 figueroa1395 commented Oct 7, 2025

In this PR I try to remove the files that aren't compliant with PGM's license from the dependencies, and then to proceed with the building of such dependencies.

The workflow is as follows: Use fossology-action (which uses fossology engine under the hood - same tooling used by LF), store the licensing results in a json file, parse that file using jq to find the files with undesirable license, remove those files, build.

Signed-off-by: Santiago Figueroa Manrique <[email protected]>
@figueroa1395 figueroa1395 self-assigned this Oct 7, 2025
@figueroa1395 figueroa1395 added feature New feature or request do-not-merge This should not be merged labels Oct 7, 2025
Signed-off-by: Santiago Figueroa Manrique <[email protected]>
Signed-off-by: Santiago Figueroa Manrique <[email protected]>
Signed-off-by: Santiago Figueroa Manrique <[email protected]>
Signed-off-by: Santiago Figueroa Manrique <[email protected]>
Signed-off-by: Santiago Figueroa Manrique <[email protected]>
@figueroa1395 figueroa1395 force-pushed the feature/correct-license-handling branch from 877e21c to be2320e Compare October 8, 2025 11:03
Signed-off-by: Santiago Figueroa Manrique <[email protected]>
Signed-off-by: Santiago Figueroa Manrique <[email protected]>
@figueroa1395 figueroa1395 force-pushed the feature/correct-license-handling branch from 5a7859e to 1c09798 Compare October 8, 2025 11:36
@figueroa1395
Copy link
Member Author

@TonyXiang8787 @mgovers Can you take a look at this. Some remarks below:

Currently, I am only doing this for Eigen and on the copied source files to test the workflow. If this way of handling this is given the thumbs up, I will then extend it to the other dependencies as well. Moreover, things can probably be deleted directly from src/pgm_build_dependencies/eigen/* and even built from there so we don't have to copy anything here; nevertheless, removing the copied header files here is also an option as this is seldomly run.

In addition, I am testing here for now since I don't have permissions to create a new repo and I can test with what is currently set up in here. But the clean repo can be created as a follow up.

Proof current works: https://github.com/PowerGridModel/pgm-build-dependencies/actions/runs/18343412484/job/52244043851. Furthermore, please take a look at the Remove files with non-accepted license step; even though I ran the same check as the LF one, I made it stricter and found a couple more potential issues, let's make sure this is fine.

One final question: Should we test we the newly built wheel against current pgm? Or the fact that we were able to build afterwards is enough proof that it works? If we want additional testing, do we manually check or do we set some test workflow to be checked before publishing is enabled?

Comment on lines 122 to 124
echo "Please check the license scan results manually."
else
echo "License cleanup completed successfully"
Copy link
Member

Choose a reason for hiding this comment

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

"check manually" implies that it should not upload. please make sure that that is indeed correctly handled

Copy link
Member Author

Choose a reason for hiding this comment

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

I meant it as something went wrong, we still upload whatever is in the scan result, take a look. Should I phrase differently?

Copy link
Member

Choose a reason for hiding this comment

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

we should upload the scan result but not do the actual committing of the files if something went wrong. we may accidentally commit something we should not

Copy link
Member Author

Choose a reason for hiding this comment

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

I will make sure then that the committing is only done if scanning was successful.

@figueroa1395
Copy link
Member Author

Up until this point, I have tested making a new release in a fork, and pip install . (*[dev], etc.) both locally and in a pgm fork (please focus on the building and testing actions only). Everything works as intended.

I will now extend this workflow to the whole repo (not only the Eigen dependency) and test in the same way as mentioned above.

Signed-off-by: Santiago Figueroa Manrique <[email protected]>
@figueroa1395 figueroa1395 removed the request for review from TonyXiang8787 October 17, 2025 08:43
@figueroa1395
Copy link
Member Author

figueroa1395 commented Oct 17, 2025

22761a3 was tested in figueroa1395#2 and https://github.com/figueroa1395/pgm-build-dependencies/actions/runs/18586672383/job/52992052911. The correct building was tested in https://github.com/figueroa1395/power-grid-model/actions/runs/18587252789 (please focus on the building only as that fork fails because it has no releases. Everything works as expected. In any case, manual run of this action is in https://github.com/PowerGridModel/pgm-build-dependencies/actions/runs/18587523173

Note: The CI in the PR doesn't tell anything, check the running action cited last above.

@figueroa1395 figueroa1395 requested a review from mgovers October 17, 2025 08:46
@figueroa1395 figueroa1395 marked this pull request as ready for review October 17, 2025 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge This should not be merged feature New feature or request

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants