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

Define //flags:lnw_version command-line flag #124

Merged
merged 2 commits into from
Apr 16, 2024
Merged

Conversation

ffoulkes
Copy link
Contributor

@ffoulkes ffoulkes commented Apr 14, 2024

  • Implemented a --//flags:lnw_version command-line flag that enables the //flags:lnw_v2 or //flags:lnw_v3 configuration setting if lnw_version is 2 or 3 and the target is ES2K.

  • Modified krnlmon_cc_library() to define the preprocessor symbol LNW_V2 or LNW_V3 if the corresponding configuration setting is enabled.


This turned out to be really tricky to do. There's an int_flag() rule, but I couldn't get it to work. I also ran into difficulties when I tried to limit the lnw_v2 and lnw_v3 conditionals to ES2K builds.

The solution turned out to be (1) write my own rule to use in place of int_flag(), and (2) use two different config_setting() parameters to and the conditionals. Fortunately, there were fairly good examples of both. The same can not be said for int_flag().

This was definitely easier to do in cmake.

- Implemented a --//flags:lnw_version command-line flag that
  defines the configuration setting //flags:lnw_v2 or
  //flags:lnw_v3 if the lnw_version is 2 or 3 and the target
  is ES2K.

- Modified krnlmon_cc_library() to define the preprocessor symbol
  LNW_V2 or LNW_V3 if the corresponding configuration setting
  is enabled.

Signed-off-by: Derek G Foster <[email protected]>
@ffoulkes ffoulkes added the bazel Affects Bazel build system label Apr 14, 2024
@ffoulkes ffoulkes added the minor effort Minimal effort required label Apr 14, 2024
@ffoulkes ffoulkes mentioned this pull request Apr 14, 2024
Copy link
Collaborator

@5abeel 5abeel left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@nupuruttarwar nupuruttarwar left a comment

Choose a reason for hiding this comment

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

LGTM

@ffoulkes ffoulkes merged commit 78bf19b into main Apr 16, 2024
4 checks passed
@ffoulkes ffoulkes deleted the lnw_version_flag branch April 16, 2024 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bazel Affects Bazel build system minor effort Minimal effort required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants