Skip to content
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

fix: track feature serialization and deserialization #1039

Merged
merged 3 commits into from
Feb 3, 2025

Conversation

baszalmstra
Copy link
Collaborator

This fixes serialization and deserialization of track features. We serialized/deserialized this as either a string or a list of strings but in fact, the "standard" is to use a space-separated string. Deserialization is backward compatible with a list. But serialization will now always produce a space-separated string.

This only affects repodata and index.json format. Lock-files will still function the same way since they have their own parsing logic, and I think a list makes much more sense.

conda/conda-build#5608

@baszalmstra baszalmstra requested a review from wolfv February 1, 2025 18:14
@baszalmstra
Copy link
Collaborator Author

Supersedes #1038

@wolfv
Copy link
Contributor

wolfv commented Feb 2, 2025

Indeed, I just tried with conda-build:

        "ttfeat-0.1.0-0.tar.bz2": {
            "build": "0",
            "build_number": 0,
            "depends": [],
            "md5": "f2dc829d0f36e4c125015591d07db7fc",
            "name": "ttfeat",
            "sha256": "f98a973b21a4943fc2872c7cb907d07e6219386509551d953b2e8062b1c37a01",
            "size": 2779,
            "subdir": "osx-arm64",
            "timestamp": 1738476674481,
            "track_features": "python tensorflow foobar",
            "version": "0.1.0"
        },

Copy link
Contributor

@wolfv wolfv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Should we call it serde::TrackFeatures instead of Features?

crates/rattler_conda_types/src/utils/serde.rs Outdated Show resolved Hide resolved
@baszalmstra baszalmstra enabled auto-merge (squash) February 2, 2025 08:47
@baszalmstra baszalmstra disabled auto-merge February 2, 2025 08:47
@baszalmstra
Copy link
Collaborator Author

LGTM! Should we call it serde::TrackFeatures instead of Features?

Its actually rhe same behavior for features. We dont parse that as a list yet because it would break the api but I thought Id already name it properly.

@wolfv
Copy link
Contributor

wolfv commented Feb 2, 2025

OK! Features are really deprecated and broken so I don't think we'll ever need them but good to know :)

@wolfv wolfv merged commit 55e0d8c into conda:main Feb 3, 2025
15 checks passed
@baszalmstra baszalmstra mentioned this pull request Feb 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants