CEP XXXX: Prefix placeholder offsets in paths.json - #179
Conversation
| modify the contents of files in an existing package (for example repackaging | ||
| or transmutation tools) MUST update the fields or remove them. | ||
|
|
||
| The fields describe occurrences of the UTF-8 encoding of the placeholder |
There was a problem hiding this comment.
This seems to be a messy edge case that I hadn't considered before an LLM noticed.
In Conda, non-UTF-8 placeholders are supported since conda/conda#9946. In rattler this isn't the case and I didn't find any discussion around this. I did find this mamba issue that seems not to have been implemented: mamba-org/mamba#2215
Now I've seen the feature, I think the non-UTF-8 encodings feature was a mistake to merge in Conda and at the least should have gone through a CEP (it was created before the CEP procedure but merged long after it was implemented). If it is going to be an officially supported feature of the conda format I think the encodings need to be standardised so this paths-offsets feature can be amended accordingly.
There was a problem hiding this comment.
If we store information about each occurrence in the file anyway, we might as well attach some additional metadata like the encoding.
For instance, another shortcoming of the current approach is listed in this issue: conda/rattler#2342. Perhaps by adding more information, we could tackle these cases as well. (See, for instance, this proposed solution: conda/rattler#2503)
There was a problem hiding this comment.
What do you think of this?
offsets becomes a dictionary:
{
"_path": "lib/libexample.so",
"path_type": "hardlink",
"file_mode": "binary",
"prefix_placeholder": "/opt/placeholder",
"offsets": {
"utf-8": [[64, 96], [200, 240, 300]]
},
"sha256": "d7a8fbb307d7809469ca9abcb0082e4f8d5651e46d3cdb762d02d0bf37c9e592",
"size_in_bytes": 4096
}The allowed keys in offsets become the POPULAR_ENCODINGS in conda. Probabaly define that the ranges MUST NOT overlap. In future special case handling for rust/go/c++ style strings could also be added with a different key (e.g. utf-8-c++).
There was a problem hiding this comment.
Yeah that could work! Or:
{
"_path": "lib/libexample.so",
"path_type": "hardlink",
"file_mode": "binary",
"prefix_placeholder": "/opt/placeholder",
"offsets": {
{ "ranges": [[64, 96], [200, 240, 300]], "encoding": "utf-8" }
},
"sha256": "d7a8fbb307d7809469ca9abcb0082e4f8d5651e46d3cdb762d02d0bf37c9e592",
"size_in_bytes": 4096
}
I think mine is easier to read, but yours is easier to parse.
While we are introducing "types" of replacements, we could also introduce one specifically for binary and one for text (e.g. bin-utf-8 or text-utf-8). That would make typing easier, maybe.
There was a problem hiding this comment.
While we are introducing "types" of replacements, we could also introduce one specifically for binary and one for text (e.g.
bin-utf-8ortext-utf-8). That would make typing easier, maybe.
I think this still needs to be discriminated on "file_mode": "binary" so I don't see the benefit of this but I might have misunderstood? Unless you're leaning towards being able to support both text and binary substitution in the same file?
There was a problem hiding this comment.
Unless you're leaning towards being able to support both text and binary substitution in the same file?
That was my thinking indeed. I don't see a use case for this now, but it would make it very easy to support that.
There was a problem hiding this comment.
I don't think this kind of semantic change can rely on an optional key. I think it would need to be a Paths v2 format with new behaviour that makes the offsets mandatory and packages unavailable to older installers. I would even consider advocating for a new virtual package so the builds are ignored by older installers that don't support the feature.
I also think the utility of mixing binary/text substitution is hypothetical, there are known common-ish use cases where non-UTF8 placeholders and non-null terminated strings are needed. I don't think I've ever come across a need for mixing text and binary mode, and even if there was I wonder if it would be better served with a patch to the sources to be more Conda friendly.
There was a problem hiding this comment.
After further thought I've prepared CEP (#180) to document the status quo which I think there is precedent for in past CEPs? It would probably then be good to have a follow up to decide what should actually be supported with prefix replacement as I clearly had a more simplistic view than what is actually in Conda these days.
Record occurrences per encoding ([{"encoding": ..., "ranges": ...}]) so a
single package serves installers with different search semantics. Producers
must now cover all five encodings replaced by existing installers; the
previous rule omitting offsets for wide-encoding files is gone. Reference
the prefix replacement CEP (conda#180) for replacement semantics and
defer padding strategies and mixed text/binary substitution to a future
versioned paths format.
The offsets CEP (conda/ceps#179) was restructured on Jul 15: `offsets` is no longer a bare list of positions but a list of offset groups, each recording the occurrences under one encoding: "offsets": [{"encoding": "utf-8", "ranges": [10, 45]}] Installers do not agree on which encodings of the placeholder they replace (conda searches UTF-8 plus UTF-16/UTF-32 variants; rattler and libmamba search UTF-8 only), so each installer applies exactly the groups its own search-based replacement covers and one package serves every installer. - `Offsets` becomes `OffsetRanges` inside a new `OffsetGroup` carrying an `OffsetEncoding`; unknown encoding names and unrecognized group members parse (they must not fail the whole paths.json) but mark the metadata as unusable. - `select_utf8_offset_ranges` validates the CEP's structural rules and picks the UTF-8 group; rattler skips the other defined encodings, whose occurrences its own search would not have replaced either. - The installer treats invalid metadata as inconsistent (falling back to search-based replacement) and copies files with no UTF-8 group through unchanged apart from text shebang handling; pre-CEP flat offsets deserialize as absent rather than failing the parse. - Adds the CEP's new test vector 9 (a binary file with occurrences under more than one encoding) and the Examples-section entries as tests.
Follow rattler_conda_types' restructure of `offsets` (conda/ceps#179): plan construction now resolves the recorded metadata once through `select_utf8_offset_ranges` — rattler applies exactly the groups its own search-based replacement covers (UTF-8 only): - usable metadata yields the UTF-8 group's ranges for the text/binary plan; valid metadata with no UTF-8 group plans zero splices (the wide string occurrences would not have been replaced by rattler's search either), serving the bytes verbatim; - structurally invalid or unrecognized metadata falls back to scanning the file, with a warning — the mount-side analogue of the installer's search-based fallback; - the selected ranges themselves remain trusted as-is, keeping the never-panic policy of the ranged reads. The macOS codesign path hands the dispatcher a synthesized UTF-8 group around the plan's c-string groups, and is skipped when there is nothing to replace (the bytes are served verbatim, so the original signature remains valid).
The offsets CEP (conda/ceps#179) was restructured on Jul 15: `offsets` is no longer a bare list of positions but a list of offset groups, each recording the occurrences under one encoding: "offsets": [{"encoding": "utf-8", "ranges": [10, 45]}] Installers do not agree on which encodings of the placeholder they replace (conda searches UTF-8 plus UTF-16/UTF-32 variants; rattler and libmamba search UTF-8 only), so each installer applies exactly the groups its own search-based replacement covers and one package serves every installer. - `Offsets` becomes `OffsetRanges` inside a new `OffsetGroup` carrying an `OffsetEncoding`; unknown encoding names and unrecognized group members parse (they must not fail the whole paths.json) but mark the metadata as unusable. - `select_utf8_offset_ranges` validates the CEP's structural rules and picks the UTF-8 group; rattler skips the other defined encodings, whose occurrences its own search would not have replaced either. - The installer treats invalid metadata as inconsistent (falling back to search-based replacement) and copies files with no UTF-8 group through unchanged apart from text shebang handling; pre-CEP flat offsets deserialize as absent rather than failing the parse. - Adds the CEP's new test vector 9 (a binary file with occurrences under more than one encoding) and the Examples-section entries as tests.
Follow rattler_conda_types' restructure of `offsets` (conda/ceps#179): plan construction now resolves the recorded metadata once through `select_utf8_offset_ranges` — rattler applies exactly the groups its own search-based replacement covers (UTF-8 only): - usable metadata yields the UTF-8 group's ranges for the text/binary plan; valid metadata with no UTF-8 group plans zero splices (the wide string occurrences would not have been replaced by rattler's search either), serving the bytes verbatim; - structurally invalid or unrecognized metadata falls back to scanning the file, with a warning — the mount-side analogue of the installer's search-based fallback; - the selected ranges themselves remain trusted as-is, keeping the never-panic policy of the ranged reads. The macOS codesign path hands the dispatcher a synthesized UTF-8 group around the plan's c-string groups, and is skipped when there is nothing to replace (the bytes are served verbatim, so the original signature remains valid).
…ental_ Per review discussion on conda#2565: until conda/ceps#179 is finalized the schema may still change, so mark the Rust API as unstable by renaming the PrefixPlaceholder fields to `experimental_offsets` and `experimental_shebang_length` (py-rattler properties follow suit). The serialized form is unchanged — serde renames keep `offsets` and `shebang_length` on the wire, as the CEP specifies. One self-contained commit so it can be reverted wholesale once the CEP merges and the fields become stable.
Follow rattler_conda_types' restructure of `offsets` (conda/ceps#179): plan construction now resolves the recorded metadata once through `select_utf8_offset_ranges` — rattler applies exactly the groups its own search-based replacement covers (UTF-8 only): - usable metadata yields the UTF-8 group's ranges for the text/binary plan; valid metadata with no UTF-8 group plans zero splices (the wide string occurrences would not have been replaced by rattler's search either), serving the bytes verbatim; - structurally invalid or unrecognized metadata falls back to scanning the file, with a warning — the mount-side analogue of the installer's search-based fallback; - the selected ranges themselves remain trusted as-is, keeping the never-panic policy of the ranged reads. The macOS codesign path hands the dispatcher a synthesized UTF-8 group around the plan's c-string groups, and is skipped when there is nothing to replace (the bytes are served verbatim, so the original signature remains valid).
Companion to the rattler_conda_types commit marking the paths.json offsets fields experimental until conda/ceps#179 is finalized. One self-contained commit so both can be reverted wholesale once the fields become stable.
The fork prefixed PrefixPlaceholder's offsets fields with experimental_ until conda/ceps#179 is finalized (serialized form unchanged). One self-contained commit so it can be reverted together with the fork's.
jaimergp
left a comment
jaimergp
left a comment
There was a problem hiding this comment.
Pretty solid. I consulted Gemini 3.6 Flash, reviewed its output and condensed the feedback in smaller suggestions. I only have some suggestions for clarity around edge cases; and a (non-blocking) proposal to reorganise the sections.
|
|
||
| ### Linking performance | ||
|
|
||
| For every path entry with a `prefix_placeholder`, installers must read the |
There was a problem hiding this comment.
| For every path entry with a `prefix_placeholder`, installers must read the | |
| For every `paths.json` entry with a `prefix_placeholder`, installers must read the |
| patched file directly from the original package data. | ||
|
|
||
| Both needs reduce to knowing, without reading the file, where the placeholder | ||
| occurs and how the file length changes. For binary files replacement preserves |
There was a problem hiding this comment.
| occurs and how the file length changes. For binary files replacement preserves | |
| occurs and how the file size changes. For binary files replacement preserves |
And same elsewhere. I find it less ambiguous to talk about size rather than length, because length makes me think of text and encodings which need to be defined.
| where the placeholder occurs in the file contents under one encoding, as | ||
| stored in the package (i.e. before any replacement). It MUST NOT be present | ||
| unless `prefix_placeholder` is present. | ||
| - `shebang_length: int`. Optional. The length in bytes of the file's first |
There was a problem hiding this comment.
How is a line defined under different encodings? For ASCII/UTF-8 is "until 0x0A is found", but for UTF-16 and above, that byte may be part of a bigger block.
| placeholder (see Rationale). | ||
|
|
||
| Its value MUST be `i + 1`, where `i` is the offset of the first newline byte | ||
| (`0x0A`) in the file, or the size of the whole file when it contains no |
There was a problem hiding this comment.
Same here, what about UTF-16 and above?
| Note that its presence does not depend on whether the first line contains the | ||
| placeholder (see Rationale). | ||
|
|
||
| Its value MUST be `i + 1`, where `i` is the offset of the first newline byte |
There was a problem hiding this comment.
| Its value MUST be `i + 1`, where `i` is the offset of the first newline byte | |
| Its value MUST be `i + 1`, where `i` is the 0-indexed offset of the first newline byte |
I assume?
| where the placeholder occurs in the file contents under one encoding, as | ||
| stored in the package (i.e. before any replacement). It MUST NOT be present | ||
| unless `prefix_placeholder` is present. | ||
| - `shebang_length: int`. Optional. The length in bytes of the file's first |
There was a problem hiding this comment.
Shall we use shebang_bytes for clarity?
| Each offset group MUST be a dictionary with exactly two keys: | ||
|
|
||
| - `encoding: str`. One of `utf-8`, `utf-16-le`, `utf-16-be`, `utf-32-le`, or | ||
| `utf-32-be`: the encodings replaced by existing installers. | ||
| - `ranges: list[int] | list[list[int]]`. The byte offsets of the occurrences | ||
| under that encoding. Its shape depends on `file_mode`, as described below, | ||
| and it MUST NOT be empty. |
There was a problem hiding this comment.
I'd fold this under the first item of the list above, as subitems. Or even better, introduce these sections:
## Specification
### Schema additions
#### `offsets`
##### Text offsets
##### Binary offsets
#### `shebang_length`
### Tooling behavior
...
| listed offset against the region boundary on every install, instead of | ||
| splicing the list uniformly. | ||
|
|
||
| ### Making text offsets relative to the end of the shebang line |
There was a problem hiding this comment.
Do we need to explicitly state that changes in the shebang length as a byproduct of replacement MUST result in the tool adjusting that difference for the absolute offsets?
| This CEP adds two optional fields to the path entries of `./info/paths.json` | ||
| (see [CEP 34](./cep-0034.md)): `offsets`, which records, per encoding, the | ||
| byte positions at which the prefix placeholder can be replaced by plain | ||
| substitution, and `shebang_length`, which marks a leading shebang line that |
There was a problem hiding this comment.
| substitution, and `shebang_length`, which marks a leading shebang line that | |
| substitution, and `shebang_length`, which marks the byte size of a leading shebang line that |
or similar
| - [conda/rattler#2032 - Implementation offsets in paths.json (original PR)](https://github.com/conda/rattler/pull/2032) | ||
| - [conda/rattler#2565 - Implementation offsets in paths.json (follow-up PR)](https://github.com/conda/rattler/pull/2565) |
There was a problem hiding this comment.
| - [conda/rattler#2032 - Implementation offsets in paths.json (original PR)](https://github.com/conda/rattler/pull/2032) | |
| - [conda/rattler#2565 - Implementation offsets in paths.json (follow-up PR)](https://github.com/conda/rattler/pull/2565) | |
| - [conda/rattler#2032 - Implementation of offsets in paths.json (original PR)](https://github.com/conda/rattler/pull/2032) | |
| - [conda/rattler#2565 - Implementation of offsets in paths.json (follow-up PR)](https://github.com/conda/rattler/pull/2565) |
?
3 participants
Opening this CEP as requested in conda/rattler#2565. AI was used to help write it but I've reviewed it extremely carefully.
Checklist for submitter
paths.json.cep-0000.mdnamedcep-XXXX.mdin the root level.Checklist for CEP approvals
${greatest-number-in-main} + 1.cep-XXXX.mdfile has been renamed accordingly.# CEP XXXX -header has been edited accordingly.pre-commitchecks are passing.