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

halcompile: Add command line arguments to provide compile and link flags #2256

Merged
merged 2 commits into from Jan 11, 2023

Conversation

andypugh
Copy link
Collaborator

@andypugh andypugh commented Jan 10, 2023

It is possible to provide extra compiler and linker flags with "option" lines inside a hal component written in the .comp preprocessor format. However for files written in straight C this does not work. This commit adds --extra-compile-args and --extra-link-args as command-line options to halcompile.
See #2204 for more background

Signed-off-by: andypugh andy@bodgesoc.org

@andypugh
Copy link
Collaborator Author

It seems that "readline" was a bad choice of library to use in this test. (This is why the test fails)

+ halcompile --userspace --compile --extra-compile-args=-I/usr/include/readline '--extra-link-args=-lm -lreadline' flags.c
        /tmp/tmpu7ipvcus/flags.c: In function 'main':
        /tmp/tmpu7ipvcus/flags.c:18:5: warning: implicit declaration of function 'rl_ding' [-Wimplicit-function-declaration]

It looks like the library is present, but the function I chose might not be in the version used by CI.

Can anyone suggest a library that is always present in a Linux installation and a function in that library that is also always present.

@jepler
Copy link
Member

jepler commented Jan 10, 2023

I applaud your effort to add a test for this, thank you!

It looks like rl_ding is not supported by the readline compatibility layer in libeditreadline, at least on bullseye.

Maybe use rl_initialize instead?

Failing that, you might put a whole self-contained useless library inside the test, including building it into a .a file.

@petterreinholdtsen
Copy link
Collaborator

petterreinholdtsen commented Jan 10, 2023 via email

It is possible to provide extra compiler and linker flags with "option" lines
inside a hal component written in the .comp preprocessor format.
However for files written in straight C this does not work.
This commit adds --extra-compile-args and --extra-link-args as command-line
options to halcompile.
See #2204 for more background


Signed-off-by: andypugh <andy@bodgesoc.org>
@andypugh
Copy link
Collaborator Author

I think that I am done with this now. The test as-is should work on any valid LinuxCNC installation and now works in the CI system.

Copy link
Collaborator

@SebKuzminsky SebKuzminsky left a comment

Choose a reason for hiding this comment

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

Looks good to me, just one question about adoc style.

This option is ignored if the option 'userspace' (see above) is set to 'no'.
When compiling a userspace component, the arguments given are inserted in the compiler command line.
* 'option extra_link_args "..."' - (default: "")
This option is ignored if the option 'userspace' (see above) is set to
Copy link
Collaborator

Choose a reason for hiding this comment

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

Doesn't this line rewrap break the "semantic line breaks" that @smoe et al have been adding?

Copy link
Contributor

@smoe smoe Jan 11, 2023

Choose a reason for hiding this comment

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

;) Just checked. Indeed. Had not seen it myself. @andypugh - the auto-translators perform differently at line breaks - not always, but often enough. To avoid that, and to help all the other languages we aim at supporting, I have worked towards individual lines to represent a continuous thought that should be translated as one unit.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I had seen the "semantic breaks" phrase without actually thinking about what it meant.
Basically lines should wrap at punctuation?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that is what it most often boils down to. And I have even introduced the one or other comma when I had felt that it would make a sentence easier to understand :)
To explain the problem a bit better, those translators aim at mimicking the overall looks of a text. And newlines are typically reserved for paragraphs in word (where they hence have a meaning). The translators then give their best to have something grammatical before and after the line break - which is not always helpful.
A nice side-effect is that the diffs in git become easier to read.

@andypugh andypugh merged commit 53406e4 into 2.9 Jan 11, 2023
@andypugh andypugh deleted the andypugh/halcompile_opts branch January 11, 2023 22:14
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.

None yet

5 participants