-
-
Notifications
You must be signed in to change notification settings - Fork 385
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
Provide dedicated sets of "extra flags" properties for platform developers and users #846
Comments
Some additional thoughts:
|
I think the number one reason for providing an easy
The Arduino IDE should provide a graphical UI for storing these Not having an obvious mechanism for injecting secrets continues to validate the old joke: "The S in IoT stands for Security". The number two reason for the |
My suggestion would be to just submit PRs adding support for the new properties to all boards platforms. It's only a matter of inserting the new properties into the compilation recipes and adding the default property definitions to platform.txt. Using a list of boards platforms I curate for this very type of project, it should be easy to write a script to set everything up for the PR automatically. Of course, a human would still need to do a check of the commit diff before submitting each PR to make sure nothing went awry.
If it's easy to implement, this information would certainly be useful. At the very least, we should document the override priorities. There are several places properties can be defined and currently no documentation of what the override priorities are (arduino/Arduino#4431). However, I would definitely not want to see this output exposed to the normal user. Property overrides are a fundamental part of how the platform system works and is used, not something that should be considered cause for a "warning". Perhaps the information could be printed in the Arduino CLI log output at either the
That would be solved by adding a user dedicated equivalent to
It's a very interesting idea! Currently, I don't have a good understanding of these configuration files. Are they using an existing format, or is this something created by Arduino? (EDIT: see https://godoc.org/github.com/arduino/go-properties-orderedmap) The |
@per1234 e.g.: |
A system for doing this has been established in the Arduino Web Editor: You could simply add the header file containing the secrets to .gitignore to preventing it from being checked in
Although I am aware of some other applications where I do agree that this capability would be useful, I'm still not convinced that the benefits outweigh the harm. I remain open minded on the subject though.
Do you think my proposal would be sufficient to make you feel that Arduino CLI is providing the "easy -D flag mechanism" you are requesting?
I think that's a fair criticism. The command line help format is a bit limiting when it comes to providing detailed documentation on complex subjects like this and the online commands documentation is generated from the command line help. In this case, I think adding the URL of the platform specification to the help text would be sufficient:
What do you think?
We already provide a mechanism for this. A macro is defined for each board according to |
@ubidefeo it's certainly possible for the platform author to add For example, a This is the ideal approach for a platform developer to offer the user the ability to define specific global macros (though typically the user would not need to have any understanding of exactly what that custom board option is doing under the hood). However, the custom board options system doesn't allow users to pass arbitrary The existing system is pretty flexible, so I think it's possible to do a lot with what we have already. When it is necessary to expand the system, I think it's best to try to make the new capabilities as much inline with the existing system as possible, so that they are relatively intuitive to the people using them. This is something I like about Alessandro's |
So I discovered through hours of trial and error that the
Hmm, I've never heard of the Arduino Web Editor. I'm someone who writes code using the vim(1) editor and uses git(1) on the command line for source control. You want me to code in a web browser, while forced to be connected to the network? I'm not sure how this is the solution to the SECRETS problem.
I think you misunderstood. I am not just looking for detecting I also write a fair number of Libraries, which have test programs (sketches) which I need to run across many different microcontrollers to make sure that they run properly on various boards. It would be impractical to run multiple dozens of validation programs manually through the Arduino IDE, manually changing various configuration options, without being able to create scripts to automate the testing.
Hiding a Additional Thoughts on the -D flag I have to say that I am constantly surprised at the push-back against the I am reaching a point where the |
Thanks for reporting this. Would you mind opening a dedicated issue report? (UPDATE: now reported at #1037)
Not at all. But note that link also explains how the system is applied to local sketches as well. I already explained how you could use that to solve your issue of needing to define secrets in a way that is not checked in to version control. However, the main reason I mention it is because we always need to consider the systems that have been previously established in the other official Arduino development tools in order to provide as consistent a user experience as possible.
I understand now. Thanks for the explanation.
OK, well this is my proposal for providing that feature. You didn't answer my question from before: Assuming it became possible to add quotes to property values set via
I'm sorry to hear that you had a bad experience using Arduino CLI. I hope that anyone who writes code will understand that bugs do happen sometimes. This project is a work in progress, hopefully steadily improving overall. High quality bug reports and feature requests are valuable contributions to that effort. |
One addition here: If we provide such a set of "user-provided flags", we should probably make sure to provide both recipe-specific flags (e.g. only for .c files, or .cpp files, or only for linking, maybe even also for the ar and elf2hex recipes) and a property to add common flags (passed to the One flag var that is missing in the first post is the one for Furthermore, I would suggest using the name What would be the next step to move this forward? I think writing a more detailed proposal about which properties a core is recommended to expose (ideally in a form that can be included in the documentation as-is)? This proposal would at least need to talk about the "user-facing" properties, but maybe it could also make some (more casual) recommendations about internal properties to use (for consistency)? |
Broken Parser in --build-properties It seems to me that the Let's say that we fix the current parser so that the comma-character is ignored inside a double-quote. That doesn't fix the parsing, because the following will not work: #define COMMA ,
#define GREETINGS {"hello", "hey", "g'day mate"} Maybe we attempt to fix that by requiring the right-side of #define NEWLINE '\n' Any attempt to add backslash-escaping of the comma ( The only way I can see Unreasonable Complexity of --build-properties My biggest issue with the The original propose in this ticket, with regards to splitting the extra flags between "platform" flags and "end-user" flags solves the problem with accidental clobbering. But I don't think it solves the problem of unnecessary complexity. I don't think the Application and Library writer should need to know that the configuration they need to override is in the I have a similar feeling about the Thoughts on arduino_secrets.h I've never used the Arduino Web Editor. If I understand the https://arduino.github.io/arduino-cli/dev/sketch-specification/#secrets page correctly, the
Summary
Given how long and how often the |
If I understand #532 correctly, it's a report of this same type of problem caused by the support for a comma-separated list of properties, and also proposes the comma-separated list support be dropped. |
Yup, that seems to be exactly this problem, so let's continue discussion about it there, and assume it will be solved somehow.
The purpose of this issue is to define a standard set of properties, so that users do not need to know the internals of the platform.txt file. If this standard property set is adopted, it can be documented in the The only caveat is that existing cores will not automatically and instantly adopt the new properties, so it might be important to let cores that support the new properties define them with empty values in platform.txt, as an indication that they support it. If so, arduino-cli could maybe provide a warning when setting a property that is not defined (with an empty value, maybe only specifically for these properties?). However, since platform.txt is the canonical source for how to run the compiler, it has to be involved when adding compiler -D options somehow, and I think there is currently no standardized attribute that the IDE/arduino-cli can define that is always included in the build. There is Hence, the only reasonable way to add flags to the commandline, is by defining a standardized set of build properties like this issue suggests. What we could maybe do, of course, is in addition to the One additional suggestion: maybe it would be good to define three sets of properties. The two suggested before (one for the platform/board developers to set internally, one for the user to set through e.g. |
This comment was marked as off-topic.
This comment was marked as off-topic.
I think that having some way to configure build flags from a sketch would be a useful addition, but I do not think that this is the right issue to dicuss this. I would want to keep this issue focused on standardizing the build properties offered by cores to allow customizing the build flags in a generic way. If we solve this issue, we can build a per-sketch-flags feature on top of it. As for the per-sketch-flags feature, I know this has been discussed repeatedly in the past, but I'm not sure if there is any currently open issue to track this? Closest I could find is arduino/Arduino#421, but that's really an old and broader issue. @per1234, do you know a better one to refer to? Coming back to the topic of this issue, I previously said:
If some way is added to define per-sketch flags, I think these could be included in the "tooling" set as well, so no extra changes are needed here to prepare for per-sketch flags later. |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
To make a clear and obvious place for "users" to modify the build configuration, why not make a For example, these existing properties:
... would become something like:
A dedicated namespace would make it clear which variables are reserved for users vs. which variables are to be used by platform vendors. Currently there is much confusion resulting from a lack of the clear delineation of variable names. To provide a concrete example, |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as duplicate.
This comment was marked as duplicate.
In theory this proposal is basically a documentation change, along with a lot of elbow grease and cat-herding to get the proposal (e.g. And it's necessary for any effort to allow project-specific definitions (e.g. arduino/tooling-rfcs#9 where I recently spelled out many use cases and why standard workarounds aren't great). The one thing I don't know how to readily manage is that different compilers may have different syntax for specifying defines ( |
That is correct.
The necessary changes to implement these properties in each platform are quite minimal, straightforward, and risk-free. The undocumented
None at all. It leverages the standard platform configuration functionality that is already in place in any Arduino development tool in use today (even in the outdated 1.x versions of Arduino IDE).
It is not relevant to this proposal. The user passes the complete arguments via the property so the user simply must use the appropriate syntax. |
So the user would need to know the syntax of the compiler in use by the platform? (And how to quote values, etc.?) |
Yes. These properties are intended to allow advanced users to do advanced things. The user must understand the compiler's command line interface before they start injecting arguments into the commands. Non-advanced users don't have any need for this functionality. If a developer is designing their project in a way that forces a novice user to use this feature, that is a mistake on their part and we will not make any effort to cater to such misuse of the feature. |
Describe the request
Establish two sets of standard "extra flags" build properties:
These should be clearly documented as being dedicated to the exclusive use by either the platform developer or the user.
The properties proposed here are only arbitrary properties, without any special use by the build system, so no changes would required in the build system. It is only a matter of setting an example for 3rd party platform developers by adding these additional properties to the
platform.txt
files of the official platforms with comments clearly explaining their intent.Ideally, the properties would be documented in the Arduino platform specification as well (#985).
Describe the current behavior
Arduino has established a convention of providing "extra flags" properties in
platform.txt
. These properties are intended to allow the compilation process to be customized by the addition of arbitrary flags in the compilation commands.For example:
https://github.com/arduino/ArduinoCore-avr/blob/1.8.6/platform.txt#L39-L49
These properties are referenced in the appropriate compilation recipes in
platform.txt
(example).The empty definitions in
platform.txt
ensure that if the extra flags are not defined elsewhere, the compilation recipes referencing them will still work. Those default empty definitions can be overrriden from any of the other platform configuration files:boards.txt
boards.local.txt
platform.local.txt
global platform.txt
or from the command line:
arduino-cli compile --build-property
arduino --pref
From the comments in the official
platform.txt
files, it seems thatbuild.extra_flags
is intended to be for the use of the boards platform developer, while the other "extra flags" properties are intended to be for the use of the user (because a platform developer has no need forplatform.local.txt
).A common use case of these properties for the user would be to add
-D
flags (e.g., #210 (comment), arduino/Arduino#6344), but there are other uses.However, platform developers sometimes use these properties in their
boards.txt
file (example). This presents the problem that the user risks inadvertently overriding the flags defined by the boards platform if they use the "extra flags" properties to modify the compilation command (example).Arduino CLI version
N/A
Operating system
All
Operating system version
Any
Additional context
Re: Possible Harmful Effect on APIs
On a certain level, this does provide the user the global macro setting capability that has been often requested, and often rejected:
boards.txt
file not used in the compilation command #210However, I think this proposal strikes the right balance of:
The concern has been that allowing users to set global macros will result in library developers unnecessarily using these as part of the API, making that API less beginner friendly (arduino/arduino-builder#15 (comment)). However, if the user is required to use a command line interface or create a specially formatted configuration file in a difficult to access location, the average user won't be interested, and so it will only make sense for a library developer to use this feature for advanced, rarely used features of the library (e.g., enable debug output for troubleshooting of the library). We have already had this "extra flags" capability since 2014, yet I haven't seen it result in any significant incorporation of user set global macros into the user-level API of Arduino libraries.
Additional requests
Related
Issue checklist
The text was updated successfully, but these errors were encountered: