-
Notifications
You must be signed in to change notification settings - Fork 5
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
Conversation
|
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. |
|
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.... |
|
@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. |
|
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
# `git@github.com:` 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 |
I don't know how to do this automatically. |
|
@Unic-X -- sorry for letting this sit. I realized that submodules are probably more trouble than they are worth. In another project of mine, I and contributors have had issues due to submodules not being properly synced and accidentally committing changes to the submodules in PRs. Such problems are very tricky to track down. In this case, we also aren't totally sure how stable this would be -- in the past, it has not worked to use a submodule for tskit-c due to the directory structure of that project. |
|
No issues at all i would try to work on something else in the meantime.
|
Solves : #182