-
Notifications
You must be signed in to change notification settings - Fork 63
Add separate flag for creating pdb files with bcc64x, enable this by … #233
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
Conversation
…default for all configurations
* docs/templates/bmake.txt:
* templates/bmake.mpd:
* templates/bmakedll.mpt:
* templates/bmakedllexe.mpt:
* templates/bmakelib.mpt:
* templates/bmakelibexe.mpt:
WalkthroughA new boolean template variable Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TemplateEngine
participant BuildConfig
User->>TemplateEngine: Selects bmake project type
TemplateEngine->>BuildConfig: Reads build configuration
BuildConfig->>TemplateEngine: Returns config with pdbl flag
TemplateEngine->>TemplateEngine: Sets LFLAGS based on pdbl
TemplateEngine->>User: Generates build files with correct linker flags
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
templates/bmakedllexe.mpt (1)
13-13: Reduce duplication: consider defaulting pdbl in a common includeTo avoid repeating pdbl = 1 in many template variants, consider setting a default in bmakecommon (or the central bmake.mpd) and overriding only where needed. Keeps future changes centralized.
Also applies to: 22-22, 32-32, 43-43
docs/templates/bmake.txt (1)
25-25: Clarify scope and fix minor spacing in pdbl doc entryTighten phrasing and note applicability. Suggested edit:
-pdbl = If this boolean template variable is set, the Linker Debugging property "Generate Program Database File" will be set according to the project target. +pdbl = If set, enables link-time generation of Program Database (PDB) files for applicable targets (executables and DLLs). Typically applies to the bcc64x toolchain and is ignored for static library targets.templates/bmake.mpd (1)
55-55: Optional: maintain backward-compatibility when pdbl is undefinedTo avoid surprising behavior for custom/legacy MPC projects that haven’t introduced pdbl yet (previously debug builds produced PDBs), consider falling back to debug_prj when pdbl is not set.
Proposed change:
-LFLAGS =<%if(pdbl)%> $(PDB_FLAGS)<%endif%> +LFLAGS =<%if(pdbl)%> $(PDB_FLAGS)<%else%><%if(debug_prj)%> $(PDB_FLAGS)<%endif%><%endif%>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/templates/bmake.txt(1 hunks)templates/bmake.mpd(1 hunks)templates/bmakedll.mpt(4 hunks)templates/bmakedllexe.mpt(4 hunks)templates/bmakelib.mpt(2 hunks)templates/bmakelibexe.mpt(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: jwillemsen
PR: DOCGroup/MPC#232
File: templates/bmake.mpd:329-338
Timestamp: 2025-08-06T12:53:40.049Z
Learning: The Embarcadero BCC64X compiler supports parallel compilation using the `{$? }` syntax in makefile pattern rules to compile all out-of-date dependencies in a single compiler invocation. This is a unique feature documented at https://docwiki.embarcadero.com/RADStudio/Athens/en/BCC64X#Parallel_Compilation and should not be flagged as incorrect when used with bcc64x.
Learnt from: jwillemsen
PR: DOCGroup/MPC#232
File: templates/bmake.mpd:329-338
Timestamp: 2025-08-06T12:53:40.049Z
Learning: The Embarcadero BCC64X compiler supports batch compilation where multiple out-of-date source files can be compiled in a single invocation using `{$? }` syntax in makefile pattern rules. This is a documented feature that combines batch compilation with parallel processing via the --jobs flag to maximize build performance. The syntax `-output-dir $(D) -c {$? }` is the correct and recommended approach for BCC64X parallel compilation.
📚 Learning: 2025-08-06T12:53:40.049Z
Learnt from: jwillemsen
PR: DOCGroup/MPC#232
File: templates/bmake.mpd:329-338
Timestamp: 2025-08-06T12:53:40.049Z
Learning: The Embarcadero BCC64X compiler supports parallel compilation using the `{$? }` syntax in makefile pattern rules to compile all out-of-date dependencies in a single compiler invocation. This is a unique feature documented at https://docwiki.embarcadero.com/RADStudio/Athens/en/BCC64X#Parallel_Compilation and should not be flagged as incorrect when used with bcc64x.
Applied to files:
docs/templates/bmake.txttemplates/bmake.mpd
📚 Learning: 2025-08-06T12:53:40.049Z
Learnt from: jwillemsen
PR: DOCGroup/MPC#232
File: templates/bmake.mpd:329-338
Timestamp: 2025-08-06T12:53:40.049Z
Learning: The Embarcadero BCC64X compiler supports batch compilation where multiple out-of-date source files can be compiled in a single invocation using `{$? }` syntax in makefile pattern rules. This is a documented feature that combines batch compilation with parallel processing via the --jobs flag to maximize build performance. The syntax `-output-dir $(D) -c {$? }` is the correct and recommended approach for BCC64X parallel compilation.
Applied to files:
templates/bmake.mpd
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: windows-2022 vs2022
- GitHub Check: ubuntu-24.04 g++-10
- GitHub Check: VS2022Release32
- GitHub Check: VS2022Debug64
- GitHub Check: macos-13-C++17
- GitHub Check: macos-14-C++17
🔇 Additional comments (8)
templates/bmakedllexe.mpt (2)
13-13: PDB generation enabled across all configs: LGTMSetting pdbl = 1 in all configurations aligns with the PR goal and the updated docs. Good default for bcc64x users.
Also applies to: 22-22, 32-32, 43-43
31-33: pdbl exclusively affects linker flags, not librarian stepsVerified in templates/bmake.mpd (line 55) that
pdblonly gatesLFLAGS = $(PDB_FLAGS)and does not appear in anylibflagsor static‐library librarian settings. No changes needed.templates/bmakelibexe.mpt (1)
12-12: Consistent pdbl enablement for lib+exe template: LGTMMatches the strategy used in other bmake templates and the documented behavior.
Also applies to: 22-22
templates/bmakedll.mpt (1)
12-12: Uniform pdbl enablement across DLL configs: LGTMConsistent with the new linker-flag gating by pdbl. No issues spotted in these blocks.
Also applies to: 21-21, 31-31, 43-43
templates/bmakelib.mpt (1)
9-9: Static library template sets pdbl: verify no unintended linker usageFor pure static libraries, PDBs are produced at the consumer link step, not by the librarian. Double-check that pdbl does not alter libflags/tlib invocations for this template.
You can reuse the script from the bmakedllexe.mpt comment to confirm pdbl is only consulted when constructing LFLAGS for EXE/DLL targets.
Also applies to: 18-18
docs/templates/bmake.txt (1)
25-25: Alphabetical placement looks correct: LGTMThe new entry is correctly positioned between obj_ext and rc.
templates/bmake.mpd (2)
55-55: Decouple PDB generation via pdbl: LGTMSwitching LFLAGS to be gated by pdbl instead of debug_prj cleanly separates “generate PDB” from “debug build.” Matches the PR objective.
55-55: pdbl is properly defined and documented
pdbl = 1is present in all intended templates:
templates/bmakedll.mpt, bmakedllexe.mpt, bmakelib.mpt, bmakelibexe.mpt- Documentation in docs/templates/bmake.txt includes a description of
pdblNo further changes required.
* docs/templates/bmake.txt:
…default for all configurations
Summary by CodeRabbit
New Features
Chores