Refactor premake dependency handling#3099
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors the Premake-based dependency configuration to reduce duplication and improve ergonomics/CI integration, including a new convenience flag to build all dependencies from source and a rename of the sqlite subproject/directory.
Changes:
- Replaced per-dependency option/processing blocks with metadata tables + loops, and added
--build-all. - Renamed the sqlite dependency/subproject from
sqlite3tosqlite(including CI + ignore rules). - Improved warning emission (GitHub Actions annotation format) and normalized some option/flag handling.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| premake5.lua | Centralizes dependency metadata/options/processing, adds --build-all, and emits GHA-style warnings. |
| premake/sqlite/premake5.lua | Renames sqlite Premake project to sqlite. |
| gframe/premake5.lua | Switches dependency include/link handling to metadata-driven loop and updates Opus/Vorbis link naming. |
| .gitignore | Updates ignored dependency folder from sqlite3/ to sqlite/. |
| .github/workflows/build.yml | Updates CI extraction paths and switches static builds to --build-all. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
Not suggesting to rename sqlite3 => sqlite
|
Reasons for using
Reasons for using
I think using |
|
https://packages.debian.org/sid/libsqlite3-dev https://vcpkg.io/en/package/sqlite3.html https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2404-Readme.md
https://sqlite.org/cintro.html
|
It don't say any 3. |
|
The prefixs of the API are |
|
The name of this subproject is SQLite. Since our project will never have multiple versions such as SQLite 2, SQLite 3, or SQLite 4 coexisting, including the version number in the subproject name serves no purpose. This is fundamentally different from how package managers handle versioning. |
Why do you think so? Just in case that you didn't notice: |
so those people are wrong |
|
I really don't want to say that if possible. You have said that I was "forcibly pushing my opinions to ygopro". |
By the way, it sounds like: |
如果你说的是“我觉得SQLite的实际名称是SQLite3,所以我们应该用SQLite3命名”,我当然会指正你的错误。这不是观点(opinions)而是对错。SQLite项目的官方名称是SQLite而不是SQLite3,这不是以我们中任何人的观点为转移的。而观点是指我们是应该用官方名称还是用什么其他的名字来命名这个子项目(的目录名和相关配置选项名)。 如果你说的是“我觉得不一定用官方项目名,应该用lib的名字”或者“应该用带版本号的名字”,我会不赞同但尊重你的观点。 |
|
以防萬一你誤會 如果你真的很排斥sqlite3 這個名字 jpg也是錯的 png也是錯的 ocgcore 也是錯的 這些目錄的名稱也應該改掉才對 |
我也同意可以不用名字去称呼一个东西,用sqlite3称呼sqlite库当然是没有问题的,但说sqlite3是 its name 我觉得不成立 |
|
我建议跳过咬文嚼字环节,关于实际的取舍:
我希望2或4中选一个 |
|
如果你覺得目錄名稱全部改掉非常麻煩 |
|
目录名使用sqlite3的话,开发者编译时还要把解压的 sqlite-amalgamation-3530300 多加一个3改成 sqlite3 |
|
libevent |
问题是需要增加异类还是减少异类。libevent 目录名带lib需要删除是1个异类,zlib目录名带lib不删除也许算一个,sqlite目录名不带3需要增加又是1个。我感觉还是减少了好。 |
|
減少異類最快也最有效的方法 |
问题是官方名称本身不够一致,也不是都能直接用于link |
目錄名稱改成libevent, libjpeg, libpng, core |
|
I prefer against changing the lib name to |
|
and it requires an adaption to https://github.com/mycard/ygopro/tree/server |
I prefer 3 |
|
and I prefer another way:
|
增加了5。继续使用 |
--build-allas a convenience option to build all dependencies from source at once--use-miniaudioand--no-use-miniaudiooptionsRename the sqlite subproject fromsqlite3tosqliteincludeorlibdirectory is not foundBug fixes:
*_INCLUDE_DIRvariables were not converted to absolute paths before being used ingframe/premake5.lua, causing relative paths supplied on the command line to be resolved relative togframeinstead of the project rootno-build-*flags were checked afterbuild-*, allowing an environment variable (e.g.BUILD_FOO=1) to silently override an explicit--no-build-fooCLI flagGetParamused Lua'sorinstead of a nil check, causing a falsy_OPTIONSvalue to incorrectly fall back to the environment variable