Opened 10 years ago

Closed 6 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:
Has a Patch: no 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)

arch-specific-capabilities.diff (2.6 KB) - added by umccullough 10 years ago.
arch-specific capabilities-based patch
0001-Add-more-capability-variables-to-OptionalBuildFeatur.patch (1.4 KB) - added by umccullough 6 years ago.
0002-Use-finer-grained-capability-indicators-for-HaikuIma.patch (5.5 KB) - added by umccullough 6 years ago.
Jamfile (4.0 KB) - added by bonefish 6 years ago.
Test Jamfile with rules for new build features syntax.

Download all attachments as: .zip

Change History (20)

Changed 10 years ago by umccullough

arch-specific capabilities-based patch

comment:1 Changed 10 years ago by korli

Good idea. Locally I had some additional arch variables like X86_AND_PPC, not really better.

comment:2 Changed 10 years ago by axeld

Indeed, looks good! I would just try to find more descriptive names, like "{HAS|HAVE|BUILD|...}_OPENGL" instead of "ENABLE_OGL".

comment:3 Changed 10 years ago by bonefish

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.

comment:4 Changed 10 years ago by umccullough

@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 in reply to:  4 Changed 10 years ago by bonefish

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 Changed 10 years ago by bonefish

BTW, for OpenSSL I already used the suggested naming scheme: HAIKU_OPENSSL_ENABLED

comment:7 Changed 9 years ago by nielx

Has a Patch: set

comment:8 Changed 6 years ago by umccullough

Owner: changed from bonefish to umccullough
Status: newassigned

comment:9 Changed 6 years ago by umccullough

Has a Patch: unset

comment:10 Changed 6 years ago by umccullough

Has a Patch: set

comment:11 Changed 6 years ago by umccullough

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

comment:12 Changed 6 years ago by bonefish

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.

Changed 6 years ago by bonefish

Attachment: Jamfile added

Test Jamfile with rules for new build features syntax.

comment:13 in reply to:  12 Changed 6 years ago by umccullough

Owner: changed from umccullough to bonefish

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 Changed 6 years ago by umccullough

Has a Patch: unset

comment:15 Changed 6 years ago by bonefish

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 Changed 6 years ago by umccullough

Resolution: fixed
Status: assignedclosed

I'd say this can be closed - now that we're successfully using it to distinguish x86 and x86_64.

Note: See TracTickets for help on using tickets.