Skip to content

chore: add tskit-c as a submodule #748

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

Unic-X
Copy link

@Unic-X Unic-X commented May 15, 2025

Solves : #182

@molpopgen
Copy link
Member

This is likely failing due to symbolic links in tskit-c. The issue of those links is why I had to remove the submodule a while back.

Further, if this is going to work, we need to pin the submodule to the precise commit as the tskit-c release that we were using before. The policy is that tskit-rust is only built against an actual tskit-c release and not to some "random" commit in that repo.

@molpopgen
Copy link
Member

I stand corrected -- the build seems stable. But you need to update the CI files to actually check out the submodule recursively.

Also need to work out which commit to use in the submodule....

@molpopgen
Copy link
Member

@Unic-X -- thanks for this. I want to think about it a bit more. I used to find it "natural" for tskit-c to be a submodule, but submodules have several drawbacks. For example, it is too easy to work on different branches of a project that are synced to different commits of the submodule and accidentally change the synced commit of a branch.

@Unic-X
Copy link
Author

Unic-X commented May 15, 2025

Yup make sense, i'll pin to the latest release's commit and test if it works.

And i guess adding submodule flag inside the CI files should fix submodule issues.

    # Whether to checkout submodules: `true` to checkout submodules or `recursive` to
    # recursively checkout submodules.
    #
    # When the `ssh-key` input is not provided, SSH URLs beginning with
    # `[email protected]:` are converted to HTTPS.
    #
    # Default: false
    submodules: ''

And for the last part, how about a new CI to check if it matches with the latest release commit of tskit-c, but i'm not sure how it would look like.

@molpopgen
Copy link
Member

And for the last part, how about a new CI to check if it matches with the latest release commit of tskit-c, but i'm not sure how it would look like.

I don't know how to do this automatically.

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