-
Notifications
You must be signed in to change notification settings - Fork 27
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
Update to reflect removal of SCSpecTypeSet #291
Conversation
Reflects stellar/rs-stellar-xdr#291 which reflects stellar/stellar-xdr#136 which addresses stellar/stellar-xdr#135 which resulted from #979 This will need updating when stellar/rs-stellar-xdr#291 merges, before it can merge.
@graydon For future reference, please don't use branch commits (e2a9cbf ) for dependencies Is there a way you can restore Alternatively, can you provide me with rs-soroban-env and SDK commits for stellar/stellar-cli#884 depending on a rs-stellar-xdr commit which is alive? |
@2opremio Yeah I'm sorry that was a mistake, not intentional. I only noticed that I left the temp commit ID in that change after it merged -- github keeps "temp commits that were part of a PR" around so CI actually passes weirdly enough -- and by the time I was repairing it Dima already had stellar/rs-soroban-env#1015 open moving forward to take the next rs-xdr change 936273b .. I didn't update stellar/stellar-cli#884 directly with that because I didn't want to step on your toes; I don't entirely understand your update process here. I think the most constructive path forward for you would be to take a newer env rev based on a non-temporary rs-xdr rev. Again I don't know exactly how you decide to take updates; it seems to involve taking a core build as well, in which case I guess I would drive your choice of which rev of env and rs-xdr to take from the core-build choice, since core lags the other two? |
I have been able to make progress after @sisuresh restored the branch. I also thought GitHub kept commits around but observed it wasn’t the case, at least with this commit, as you can see at https://github.com/stellar/soroban-tools/actions/runs/6010764494/job/16302863738?pr=884 |
GitHub keeps commits, but git won't fetch them by default. Unless your git repository previously fetched it, if a commit is not referenced by a branch you'll need to fetch it manually with |
Well, that breaks cargo as per https://github.com/stellar/soroban-tools/actions/runs/6010764494/job/16302863738?pr=884 |
It is of vital importance for the core team to understand that, in order for platform (soroban-tools) to consume an update we need
|
Well, I did understand that at a conceptual level, but maybe not quite thoroughly enough in terms of the practice of proposing a partial change to tools as I did. Three things I think went wrong in this cycle:
I think the question of "when the core team is obliged to produce such a synchronized 5-tuple" / "when the platform team will expect to have one to accomplish a successful tools update" is worth addressing, and is mostly what I meant above by "I don't know how you decide to take updates". Like maybe you only want them sometimes, so us being out-of-sync for a few days at a time is ok since you only want to do integration runs once a week or so. Or maybe you to do an update every time any one of the 5 repos changes. I don't entirely know. We're obviously failing to meet expectations currently, as you're having your days wasted hitting roadblocks. I'm sorry about that. A policy around this might make expectations clearer and reduce frustration. I'd be happy to follow any explicit policy, so long as both teams' managment is ok with the impact on schedules (more frequent and wider synchronization means a bit more overhead / slower progress in each repo, but maybe that's ok if it lets the teams stay in closer sync). Earlier in the project I think @leighmcculloch managed more of these handoffs, especially the SDK step you're seeing us lagging on (core doesn't depend on it), and probably it's not so good to rely on him as human shock absorber. |
(Another question I feel is pertinent here, and I don't mean to unload work on you by asking, but I think it might be worth discussing: are you ever comfortable / would you like to have rights to push updates to SDK if we've failed to update it, i.e. when it's a revision or two behind the more closely-coupled xdr + rs-xdr + env + core set? I see you've done so in the past. I know part of the problem here is timezones and workdays not overlapping enough between teams, and I think it would probably be fine to expand SDK's committer set from stellar/contract-committers to include stellar/horizon-committers also -- and maybe others, there's quite a wide set of writers on soroban-tools -- since the SDK is really more of a bridge repo between teams and its versioning policy is potentially a little more flexible. I don't know for sure though, @MonsieurNicolas and @leighmcculloch wdyt?) |
Thanks for making this explicit. I see requests for a "core build" from the Core team in Slack. I think we need to keep being explicit and own what we need rather than assume other teams will remember which components we need. Everyone manages multiple components and it's trivial to forget another team needs specific components. When requesting artifacts prior to a release, ask for all the artifacts needed, which I think for Platform is more than a core build and typically: stellar-core, stellar-xdr, soroban-env, soroban-sdk.
I don't think changing who is responsible for updating the SDK will help this situation. Doing so may make coordination and the interface between teams harder to navigate. When planning for a pre-release cut of products, we might benefit from treating it much like a release event, possibly even target a tag that doesn't get published to crates.io. If we have a page similar to the releases docs page for these internal releases, it might help catch missing components earlier. There's enough to coordinate that I think it warrants the process. Wdyt @graydon? @anupsdf?
This mistake is easy to make, and we've made the same mistake before. We may be able to add a CI check to the |
In general, every time soroban-tools is updated with respect to Core (yeah, I know this isn't very helpful) In practice, this happens every time there is a (substantial) XDR update. As a rule of thumb, whenever we (platform) are awaiting for a new Core build (@sisuresh has been providing these) we also need the rs-env commit and a matching SDK update.
Yes, I agree. An official policy on how things should be updated would help. I was hoping this wouldn't be necessary but after having reached this point, it probably is. |
Having write permissions to the Rust repos would help (e.g. it would had allowed me to restore the branch pointing to the dangling commit here and move forward) However, in the particular case of the SDK, it can be more complicated than simply bumping it's xdr/rs-env dependencies to the desired ones. In this case, the SDK had already "overtaken" the rs-env dependency we were targetting, requiring a downgrade. We didn't want to downgrade the main repo so @sisuresh created a separate branch in his fork https://github.com/sisuresh/rs-soroban-sdk/tree/ee389 . As a result, soroban-tools (even if temporarily) now depends on @sisuresh 's fork, which is less than ideal I don't want to find myself downgrading the SDK. |
The issue in this specific case was that the core team was making a lot of changes while we were trying to get a build out, and the result was that the sdk was bumped to a version of env that was past what the build used. Going forward, our policy should just be that every env change should be followed by an sdk change (and if that's too burdensome, make sure every core build we give to platform has a matching sdk commit). We can mitigate mistakes here by always giving platform the core version, env hash, and the sdk hash. |
Reflects stellar/stellar-xdr#136 (and will need a refresh after that merges to bump the XDR ref)