Opened 16 years ago
Closed 12 years ago
#3798 closed enhancement (fixed)
[PATCH] Arch-specific capabilities patch for HaikuImage
Reported by: | umccullough | Owned by: | bonefish |
---|---|---|---|
Priority: | normal | Milestone: | R1 |
Component: | Build System | Version: | R1/pre-alpha1 |
Keywords: | Cc: | ||
Blocked By: | Blocking: | ||
Platform: | All |
Description
Rene and I were brainstorming a solution for defining targets that apply to more than one arch, but not all, and Rene suggested a "capabilities-based" system as suggested in the attached patch.
Basically it defines each capability that may be arch-specific and uses that to determine whether it can be built or not.
Currently OpenGL is builds for x86 and ppc, but not necessarily m68k or others. Likewise, libavcodec currently builds for x86, and *almost* builds for PPC, and probably will build for SPARC in the future as well.
Attachments (4)
Change History (20)
by , 16 years ago
Attachment: | arch-specific-capabilities.diff added |
---|
comment:1 by , 16 years ago
Good idea. Locally I had some additional arch variables like X86_AND_PPC, not really better.
comment:2 by , 16 years ago
Indeed, looks good! I would just try to find more descriptive names, like "{HAS|HAVE|BUILD|...}_OPENGL" instead of "ENABLE_OGL".
comment:3 by , 16 years ago
Maybe a more global place could be chosen to define what features can be built, so that these variables are also available in the Jamfiles. Then I would change the variable names to move them into the global namespace, e.g. HAIKU_OPENGL_ENABLED.
follow-up: 5 comment:4 by , 16 years ago
@axeld: yes, we did discuss a couple different ideas such as HAS_* or OPENGL_ENABLED, etc. I can certainly expand the variables out to something more descriptive on the next try.
@bonefish: I will rework the patch to use something more global - any suggested locations off the top of your head? Otherwise I'll try to find somewhere suitable this evening after work.
Thanks for the feedback!
comment:5 by , 16 years ago
Replying to umccullough:
@bonefish: I will rework the patch to use something more global - any suggested locations off the top of your head?
I'd introduce a build/jam/BuildFeatures, included before UserBuildConfig.
comment:6 by , 16 years ago
BTW, for OpenSSL I already used the suggested naming scheme: HAIKU_OPENSSL_ENABLED
comment:7 by , 15 years ago
patch: | → 1 |
---|
comment:8 by , 12 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:9 by , 12 years ago
patch: | 1 → 0 |
---|
by , 12 years ago
Attachment: | 0001-Add-more-capability-variables-to-OptionalBuildFeatur.patch added |
---|
comment:10 by , 12 years ago
patch: | 0 → 1 |
---|
by , 12 years ago
Attachment: | 0002-Use-finer-grained-capability-indicators-for-HaikuIma.patch added |
---|
comment:11 by , 12 years ago
Are these two patches closer to a preferred solution?
I didn't necessarily like adding the local <feature>_ONLY variables in HaikuImage, but it seemed the more sane solution given that the HAIKU_<feature>_ENABLED globals are set to a value of 1 in this case.
Another option is to create conditional sections in HaikuImage to add extra targets?
Note: we may want to find a nice solution for this problem before x86_64 is merged into HaikuImage
follow-up: 13 comment:12 by , 12 years ago
The reason why I suggested a new build/jam/BuildFeatures is that those aren't really optional in that they represent features one can build with or without (e.g. SSL). Instead their availability depends merely on the architecture. Admittedly this line is rather blurry, since some stuff is provided as an optional package only for some architectures, and, even worse, AFAIK some stuff isn't even optional at all (ICU). So it might be better to just rename OptionalBuildFeatures to BuildFeatures and put the declarations for all the features in there.
Anyway regarding the actual solution for HaikuImage, I think that the *_ONLY
approach has outgrown its originally simple elegance. Instead of a $(X86_ONLY)
tag here and there we have those tags in a lot of places now, and with x86-64 coming into the fold and maybe ARM soon as well there'll be even more. We won't get around declaring what target is enable for which architecture/feature, but I was thinking of a syntactically more elegant solution.
I'm attaching a test Jamfile with the rules needed for that solution. Some examples for the interesting ones:
EnableBuildFeatures opengl ffmpeg freebsd_network : x86 x86_64 ; EnableBuildFeatures ssl : !arm ;
The first argument to EnableBuildFeatures
is the list of features to enable, the second is a list of conditions. If no conditions are given, the features are enabled unconditionally, otherwise only when the conditions hold true. The condition list can consist of any number of positive and negative ('!' prefix) conditions. The features are enabled, when any of the positive conditions and none of the negative conditions hold true. The condition elements are build features themselves. A positive/negative condition holds true when the respective build feature was/wasn't enabled before. The architecture is initially added as a build feature unconditionally (EnableBuildFeatures $(HAIKU_ARCH) ;
), so the above examples work.
When a feature is enabled its name is added to the variable HAIKU_BUILD_FEATURES
and a variable HAIKU_<FEATURE>_ENABLED
is defined (<FEATURE>
is all upper case).
The second rule is FMatchesBuildFeatures
. It returns "1" (i.e. true), if the given condition holds, or an empty list (i.e. false) otherwise. It supports the same condition lists as EnableBuildFeatures
and additionally a comma-separated form.
if [ FMatchesBuildFeatures x86 x86_64 ] { # x86 or x86-64 } if [ FMatchesBuildFeatures opengl,!arm ] { # OpenGL but not ARM }
The most interesting rule for HaikuImage is FFilterByBuildFeatures
. It takes a list annotated with build feature conditions and returns a list filtered accordingly.
AddBootModuleSymlinksToHaikuImage [ FFilterByBuildFeatures x86,x86_64 @{ acpi isa ide_isa }@ openpic@ppc ata @{ ata ata_adapter }@ ide @{ ide ide_adapter }@ pci config_manager dpc locked_pool scsi_periph scsi usb ahci generic_ide_pci it8211 legacy_sata silicon_image_3112 <usb>uhci <usb>ohci <usb>ehci scsi_cd scsi_disk usb_disk efi_gpt intel bfs ] ;
There are two kinds of annotations. Individual list elements can be annotated (e.g. openpic@ppc
-- openpic
only when ppc
is enabled) and groups of elements can be annotated by enclosing them in @{ ... }@
preceded by the condition string. Nesting those groups is possible as well as using element annotations within a group. For an element to be included in the filtered list its own condition (if any) and the condition of every containing group has to hold.
All *ToImage
(or generally *ToContainer
) rules could filter their targets argument list by default, which would allow us to omit the explicit invocation of FFilterByBuildFeatures
when the list is passed to such a rule directly (as in the example above).
The exact syntax is only a proposal. If anyone has a nicer idea, let's hear it. I picked the @
symbol as a separator for the element annotation, since it is a symbol that's rather unlikely to appear in regular target names. The annotation is a suffix, since it felt more natural at first. Though with the group annotation condition being a prefix, that could be reconsidered. Also for the group condition everything is separated by spaces while no spaces are used for the element annotations. Admittedly openpic @ ppc
is nicer to read than openpic@ppc
. OTOH, in a single-line list it's clearly the other way around (foo@x86 bar@ppc foobar@arm
vs. foo @ x86 bar @ ppc foobar @ arm
).
For grouping using something with matching opening and closing symbol is nice. The obvious simple pairs ( )
, [ ]
, { }
, < >
can't be used, since they are special jam tokens and would have to be quoted. Due to @
being used for the element annotations @{ }@
seemed a reasonable choice. Though other alternatives -- e.g. @x86,x86_64( ... )@
-- would look just as reasonable.
Opinions and suggestions welcome.
comment:13 by , 12 years ago
Owner: | changed from | to
---|
Replying to bonefish:
Opinions and suggestions welcome.
Wow, that's a lot more involved than I expected. I'll see if I can find some time soon (weekend) to digest what you've wrote and attached and see if there's anything I can add to push this along.
Assigning this back to you as you clearly have thought this through a lot more than I.
comment:14 by , 12 years ago
patch: | 1 → 0 |
---|
comment:15 by , 12 years ago
I committed my proposed changes (with small adjustments) in hrev44986. So the framework is in place. Now it should only be a matter of adding/adjusting the respective annotations. I'll leave that to whoever is interested.
comment:16 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
I'd say this can be closed - now that we're successfully using it to distinguish x86 and x86_64.
arch-specific capabilities-based patch