10000 [Feature] Added features to the premake cli wrapper by Ohjurot · Pull Request #17398 · conan-io/conan · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[Feature] Added features to the premake cli wrapper #17398

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Feb 13, 2025
Merged

[Feature] Added features to the premake cli wrapper #17398

merged 7 commits into from
Feb 13, 2025

Conversation

Ohjurot
Copy link
Contributor
@Ohjurot Ohjurot commented Dec 1, 2024 •
edited by czoido
Loading

Changelog: Feature: Enhanced Premake CLI wrapper with configurable Lua file path, and support for custom command-line arguments.
Docs: conan-io/docs#4000

  • premake5 action (vsXXXX or gmake2) gets auto-detected in constructor but can be changed via public member.
  • The path and filename to the root premake5.lua file is changeable
  • Custom premake5 command line arguments can be specified via public dict arguments

Thees changes are required in order to fully support calling the premake5 executable. This lays the foundation for a PremakeToolchain.

This is one of the key components of #14724 but in a modified version. I plan to add the other main feature (build) as a follow up under PremakeToolchain. As I have understand as long as premake does not have a build feature. Which it does not have. It should be "chained" as a Toolchain that would utilize the cli wrapper and the platform specific build system wrapper (MSBuild or a direct make call) other than being integrated in the cli wrapper. Correct?

Would we need to add any test coverage other than the "basic" test @memsharded added? Like checking that arguments work etc.)

Currently there is (as previously discussed) no documentation for the whole premake feature set. I assume we will stay like this for now. I would however recommend to document the feature as soon as premake is feature complete.

  • Refer to the issue that supports this Pull Request.
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

@Ohjurot
Added features to the premake cli wrapper
a3b5cf6 
- Modifiable premake action (autodetected in constructor)
- Modifiable lua file location
- Custom user actions (defined within the premake5.lua file) exposed as 'arguments'
@Ohjurot Ohjurot mentioned this pull request Dec 1, 2024
Closed
5 tasks
memsharded
memsharded reviewed Dec 15, 2024
View reviewed changes
Copy link
Member
@memsharded memsharded left a comment

Choose a reason for hiding this comment

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

It would be great to add a couple of unit test illustrating the usage and functionality of these changes. I understand they cover some customization cases, which seems legit, but seeing the actual usage tests help to polish the UI and also to get them covered for the future to avoid possible future breakages.

Thanks for your contribution @Ohjurot !

@Ohjurot
Copy link
Contributor Author
Ohjurot commented Dec 19, 2024

Thanks for the feedback @memsharded! I will implement as soon as I have time.

@memsharded
Copy link
Member

Hi @Ohjurot

It would be nice to get this merged, do you think you will be able to add a couple of small tests?
If not let me know and I will try to add them myself. Thanks!

@Ohjurot
Copy link
Contributor Author
Ohjurot commented Feb 12, 2025 •
edited
Loading

Hi @memsharded
Sorry for the delays. I’m currently quit constraint in time. So I would be really grateful if you could help me out with the tests. I would also appreciate a quick reference to the „ideal“ tests that the full integration would need. The I can directly work on that in the next contribution step, when I have time.
Thank you!

@memsharded memsharded added this to the 2.13.0 milestone Feb 12, 2025
@memsharded memsharded mentioned this pull request Feb 12, 2025
Merged
@memsharded
Copy link
Member
memsharded commented Feb 12, 2025

Taking over the tests, I am also provisioning premake in CI in #17765

@memsharded memsharded merged commit b6788ad into conan-io:develop2 Feb 13, 2025
34 checks passed
@memsharded
Copy link
Member

Merged, it will be in next Conan 2.13, thanks again for the contribution

@czoido czoido mentioned this pull request Feb 25, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@memsharded memsharded memsharded left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.13.0
Development

Successfully merging this pull request may close these issues.
2 participants
@Ohjurot @memsharded
0