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

redesign dependency-list creation for github prepare action #6255

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hgy59
Copy link
Contributor

@hgy59 hgy59 commented Oct 5, 2024

Description

This redesign speedsup the evaluation of dependent packages in github prepare action.
Instead of 3 to 5 minutes is now takes only a few seconds create the dependency list that is required.

  • add script to evaluate the dependencies
  • use new script in global Makefile for the dependency-list target
  • use global Makefile to create dependency-list in prepare.sh
  • avoid variables in DEPENDS definitions
  • make dependency-list.sh executable

Mandatory framework changes

  • makefile variables like $(SPK_NAME) are not allowed in dependency definitions anymore

Checklist

  • Build rule all-supported completed successfully
  • New installation of package completed successfully
  • Package upgrade completed successfully (Manually install the package again)
  • Package functionality was tested
  • Any needed documentation is updated/created

Type of change

  • Includes framework changes

- add script to evaluate the dependencies
- use new script in global Makefile for the dependency-list target
- use global Makefile to create dependency-list in prepare.sh
- avoid variables in DEPENDS definitions
- make dependency-list.sh executable
@hgy59 hgy59 added the framework label Oct 5, 2024
@hgy59
Copy link
Contributor Author

hgy59 commented Oct 5, 2024

@th0ma7 I wand to remove some obsolete dependencies.

Can't we remove native/nasm and native/yasm since those are prerequisites for the development environment (and included in the spksrc docker image)?


The same applies to native/cmake but here you have recent updates to intel-vc-intrinsics that needs USE_NATIVE_CMAKE, so I guess we must keep native/cmake.

Another one is native/cmake-legacy. This one is used with USE_NATIVE_CMAKE_LEGACY and is used for mysql-connector-c only.
Probably we can drop the mysql-connector and use mariadb-connector with mysql compatibility (CMAKE_ARGS += -DWITH_MYSQLCOMPAT=ON) instead?

@th0ma7
Copy link
Contributor

th0ma7 commented Oct 6, 2024

Can't we remove native/nasm and native/yasm since those are prerequisites for the development environment (and included in the spksrc docker image)?

yes certainly. although a fair bit of testing may be needed along with adapting the framework. Just to manage expectations, I can certainly review code but my next focus really needs to be python...

The same applies to native/cmake but here you have recent updates to intel-vc-intrinsics that needs USE_NATIVE_CMAKE, so I guess we must keep native/cmake.

That's probably an oversight as I updated our debian image during the time I was workding on intel opencl and this may have be there while using the older image. I'll check if that can be removed.

Another one is native/cmake-legacy. This one is used with USE_NATIVE_CMAKE_LEGACY and is used for mysql-connector-c only. Probably we can drop the mysql-connector and use mariadb-connector with mysql compatibility (CMAKE_ARGS += -DWITH_MYSQLCOMPAT=ON) instead?

Feel free to remove that altogether. I've been trying to keep mysql-connector-c alive as prerequisite for other packages that i ended-up migrating, mainly related to python if i recall. There used to be chromaprint but I believe it's now solved.

Although my 2 cents are, cmake native and legacy have proved to be quite useful. I'd be tempted to keep the code for newer cmake as i'm sure we'll hit this again later in a year or two.

Also, I believe it would be time to start stripping off broken packages that never received any love since pre-DSM6, along with deleting them from our repository online. As well, I would remove all the older toolchains as no longer needed and clean-up the code related to tc and tk accordingly. This would reduce the framework weight for a bit and accelerated archs evaluation.

@hgy59
Copy link
Contributor Author

hgy59 commented Oct 6, 2024

@th0ma7 thank you for the feedback.

I think I will hold back this PR for prior creating some smaller PRs for:

  • removal of variables in DEPENDS definitions (multiple PRs to touch less packages in each)
  • removal of native/nasm native/yasm
  • removal of python2
  • ...

Then, later on, in this PR:

  • use scripts for all dependency lists (dependency-list and dependency-flat)
  • remove dependency-tree. It is not very usefull (not readable) with such a lot of dependencies. It was the only source for dependency evaluation before we introduced dependency-list.

Finally, to remove OPTIONAL_DEPENDS, I would prefer separated PRs again.

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.

2 participants