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

Invoke "gcc" to build assembler files #671

Merged
merged 1 commit into from
Jan 24, 2024
Merged

Conversation

jld01
Copy link
Contributor

@jld01 jld01 commented Jan 14, 2024

Enables pre-processing of *.S and *.sx source files.

Part of #666

@jld01
Copy link
Contributor Author

jld01 commented Jan 14, 2024

I am hopeful that this patch will address concerns about the migration of existing projects when invoking gcc rather than as to build assembly files in managed build projects. My personal opinion is that a switch to using gcc is justified here because the current behaviour is not aligned with user expectations for building *.S and *.sx files.

I have added a command generator for the Assembler flags MBS option that looks for cases where the tool command is gcc but the assembler flags option does not include the -c flag. In such cases:

  • The -c flag is added
  • Other assembler flags that are not automatically passed to the assembler by the gcc wrapper are prefixed with -Wa,

For example, a project that was previously building assember files using as -K -g would now use gcc -c -Wa,-K -g.

@jld01 jld01 force-pushed the gcc-assembler branch 5 times, most recently from d00a3a9 to 6aacb7e Compare January 14, 2024 17:43
@ruspl-afed
Copy link
Member

ruspl-afed commented Jan 14, 2024

Looks good for the simple case. And this covers part of the issues raised during the CDT discussion.

And what about [so nice] MBS inheritance? There are three value changes that can participate to the inheritance chain:

  • Tool "command"
  • Tool "commandGenerator"
  • Option "defaultValue"

How do we ensure that this fix will not break downstream integrations?

@jld01
Copy link
Contributor Author

jld01 commented Jan 16, 2024

Thank you the feedback, @ruspl-afed.

And what about [so nice] MBS inheritance? There are three value changes that can participate to the inheritance chain:

* Tool "command"
* Tool "commandGenerator"
* Option "defaultValue"

How do we ensure that this fix will not break downstream integrations?

The latest version of this PR accommodates cases where the tool command has been overridden XOR the option default value has been overridden. There is no issue when both have been overridden. There is no issue when neither has been overridden.

This work does not accommodate the possibility that the tool command generator has been overridden while the tool command and option default value have not been overridden. The risk here seems very low.

@jld01 jld01 marked this pull request as ready for review January 16, 2024 20:03
@jld01 jld01 requested a review from jonahgraham January 17, 2024 08:19
@jld01
Copy link
Contributor Author

jld01 commented Jan 17, 2024

@jonahgraham I've invited you to review since there has been some difference in opinion on this topic. I would suggest a N&N entry that covers "how to fix" in extreme edge cases such as overridden Assembler flags option command generator.

@jld01 jld01 force-pushed the gcc-assembler branch 2 times, most recently from b968366 to c067ac8 Compare January 20, 2024 10:06
Enables pre-processing of *.S and *.sx source files.
@jld01 jld01 merged commit 233c6d8 into eclipse-cdt:main Jan 24, 2024
5 checks passed
@jld01 jld01 added this to the 11.5.0 milestone Jan 24, 2024
try {
if (null != toolCommand && IOption.STRING == option.getValueType() && option.getCommand().isEmpty()) {
String optionValue = option.getStringValue();
if (toolCommand.equals("gcc")) { //$NON-NLS-1$
Copy link
Member

Choose a reason for hiding this comment

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

Sorry to be late the party here, but should this line not match arm-non-eabi-gcc for example, or gcc.exe? Or is it enough to have a strict check for gcc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are checking only whether the tool command has been overridden. We don't care if it has been overridden by the user or by an extender. If the tool command has been overridden, then our new default value (gcc) will have no effect on the build and we do not need to modify the assembler flags.

@jld01 jld01 deleted the gcc-assembler branch January 29, 2024 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants