Skip to content

Simplify mapping of target properties - #755

Open
WhyNotHugo wants to merge 1 commit into
lexiforest:mainfrom
WhyNotHugo:libs-mapping
Open

Simplify mapping of target properties#755
WhyNotHugo wants to merge 1 commit into
lexiforest:mainfrom
WhyNotHugo:libs-mapping

Conversation

@WhyNotHugo

Copy link
Copy Markdown

libs.json declares objects with sets of properties for each supported target. Most of them repeat information, since most properties depend on a single value.

For example, 'obj_name' only depends on 'system', and doesn't need to be repeated for each target. The same is true of most other properties.

Adding new targets requires repeating all the information for each one.

Simplify this logic by resolving properties individually. This change implicitly adds support for building on numerous new targets, including musl-loongarch64, musl-armhf, musl-i686, etc. Supporting additional targets becomes trivial, since only the new variables need to be declared.

Checklist

  • I have manually reviewed the changes and fully understand the code.

@WhyNotHugo

WhyNotHugo commented May 8, 2026

Copy link
Copy Markdown
Author

CI fails with:

Error: You must check the box: 'I have manually reviewed the change and fully understand the code.'

This needs to be checked by a reviewer, not by the author, correct?

libs.json declares objects with sets of properties for each supported
target. Most of them repeat information, since most properties depend on
a single value.

For example, 'obj_name' only depends on 'system', and doesn't need to be
repeated for each target. The same is true of most other properties.

Adding new targets requires repeating all the information for each one.

Simplify this logic by resolving properties individually. This change
implicitly adds support for building on numerous new targets, including
musl-loongarch64, musl-armhf, musl-i686, etc. Supporting additional
targets becomes trivial, since only the new variables need to be
declared.
@WhyNotHugo

Copy link
Copy Markdown
Author

v2: added armhf/armv8l

@lexiforest

Copy link
Copy Markdown
Owner

The flattened structure is intended. It is redundant at first sight, but that's also the benefit itself, we don't need to worry about breaking other things when editing one of them.

@lexiforest

Copy link
Copy Markdown
Owner

The philosophy behind the redundancy is the rule of three.

@WhyNotHugo

WhyNotHugo commented May 15, 2026

Copy link
Copy Markdown
Author

I'm not sure I see what you mean. What could break with my proposed design?

The rule implies that the cost of maintenance outweighs the cost of refactoring and potential bad design when there are three copies, and may or may not if there are only two copies.

Without this change, support for musl would require septuplicating all the definitions.

Considering that this is data rather than code, I'd consider normalisation the rule which applies here: in particular, this change aligns closer with Second/Third normal form.

@WhyNotHugo

Copy link
Copy Markdown
Author

See #763 as an approach extending musl support with the current design.

WhyNotHugo added a commit to WhyNotHugo/curl_cffi that referenced this pull request May 15, 2026
@WhyNotHugo

Copy link
Copy Markdown
Author

FWIW, we're including this patch downstream on Alpine to build packages for
musl-aarch64, musl-armhf, musl-armv7, musl-loongarch64, musl-riscv64, musl-x86
and musl-x86-64.

ppc64le and s390x are excluded for now since libcurl-impersonate lacks support
for them.

@lexiforest lexiforest added the needs more info This bug report is not complete label Jun 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs more info This bug report is not complete

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants