-
Notifications
You must be signed in to change notification settings - Fork 70
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
user: rewrite parts of the Build Constraints chapter #323
Conversation
b68647d
to
96a0760
Compare
Fixes: #291 Signed-off-by: Nathan Cutler <[email protected]>
96a0760
to
afda1d4
Compare
<para> Constraints can be set for a particular package by including a | ||
_constraints file in the package. </para> | ||
<para> Constraints can also be included directly in the build recipe (spec, | ||
dsc, etc.), provided the build recipe parser supports it. </para> |
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.
It would be nice if you would name the ones we know support it right now...
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.
it is written there already: "Example for rpm spec file or Dockerfile:"
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.
I can write expicitly that only spec and Dockerfile are supported, but then the moment support is expanded to another recipe that statement will be wrong...
In general I would also mention how to get an idea what a build is using through the https://build.opensuse.org/package/statistics/home:hennevogel/gifup/openSUSE_Tumbleweed/x86_64 and the API |
<para>It's not clear how the reader would find out if a given build recipe | ||
parser supports BuildConstraints and it would even be nice here to link to a | ||
discussion of what, specifically, is meant by "build recipe". </para> | ||
</warning> |
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.
^^^ @hennevogel
Instead of naming here the recipe parsers that support (and then dooming ourselves to the list being incomplete when the developers expand the support to more and more parsers), perhaps it would be better to empower the reader to find out the actual state.
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.
no, please don't write such a thing in the documentation. The documentation is here to explain that.
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.
Please note, this is just a draft and these warnings will disappear before the "draft" flag is removed. Just to be clear.
(I used <warning>
because I assume docbook doesn't have a <fixme>
;-)
Yes, the idea to fix #291 is what got me started on this, but I found that (if you'll forgive me comparing the fix to an airplane) the fix had no nice place to land. So I started building an airport... |
On Freitag, 9. Februar 2024, 10:13:39 CET Nathan Cutler wrote:
@smithfarm commented on this pull request.
> - starting the bs_worker process. It can be used to run on specific hosts,
- which may be used for running benchmarks in a reproducible way. This
- constraint can also be defined as a negative definition using the
- <literal>exclude=true</literal>
- attribute. However, the hostlabel is always specific to one OBS instance.
- You should avoid it as much as possible, since building with this constraint
- in another instance is usually not possible. Use any of the other
- constraints if possible. </para>
- <para> Example for _constraints file:</para>
- <screen><constraints exclude="false">
- <hostlabel>benchmark_runner</hostlabel>
+ <title>Setting constraints</title>
+ <para> Constraints can be set for a particular package by including a
+ _constraints file in the package. </para>
+ <para> Constraints can also be included directly in the build recipe (spec,
+ dsc, etc.), provided the build recipe parser supports it. </para>
I can write expicitly that only spec and Dockerfile are supported, but then the moment support is expanded to another recipe that statement will be wrong...
yes, that is our reality that we always need to run after the documentation
on implementation :)
…--
Adrian Schroeter ***@***.***>
Build Infrastructure Project Manager
SUSE Software Solutions Germany GmbH, Frankenstraße 146, 90461 Nürnberg, Germany
(HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev
|
Documentation is part of the implementation... |
@adrianschroeter I understand the sentiment and that this is the current reality, but it doesn't have to be that way. I can imagine an API call |
On Freitag, 9. Februar 2024, 13:09:56 CET Nathan Cutler wrote:
> yes, that is our reality that we always need to run after the documentation on implementation :)
@adrianschroeter I understand the sentiment and that this is the current reality, but it doesn't *have* to be that way. I can imagine an API call `/buildconstraints/supported/buildrecipeparsers` which would return the real answer to the question "which recipe parsers support buildconstraints?" And then the documentation could tell folks how to get the real answer, instead of presenting a false answer.
This is not an api feature, but a build process tooling support task.
And even when you have that, you would still need to document
the specific syntax for each build description format.
…--
Adrian Schroeter ***@***.***>
Build Infrastructure Project Manager
SUSE Software Solutions Germany GmbH, Frankenstraße 146, 90461 Nürnberg, Germany
(HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev
|
OK, this discussion has helped me and I now know how to deal with this "supported build recipes" problem. Wait for it ;-) |
will open a clean PR |
Fixes: #291