chore: rename FUSER fork as mountpoint-s3-fuser #1283
Conversation
ujinho
force-pushed
the
fuser/fork-crate
branch
2 times, most recently
from
5128e8a to
684c906
Compare
ujinho
force-pushed
the
fuser/fork-crate
branch
from
684c906 to
12a9e99
Compare
ujinho
force-pushed
the
fuser/fork-crate
branch
from
12a9e99 to
d97b4ba
Compare
There was a problem hiding this comment.
I think we want to flip the way we're doing the updates.
My thinking is that this should be the process at a high-level when consuming an update or adding a new patch:
- Take
fuser/forkbranch and rebase it on top of fuser upstream, if wanted. - Add any new commits.
- Push a tag for the new fork version.
- Create a PR on the
mainbranch that updates the submodule to point to the new tag. - (Later, when publishing the crate, we'll also push a tag. We should ensure that there's no confusion with those and the ones we maintain to avoid GC on the fork.)
That last step ensures that we always have a tag pointing to those commits, and doesn't rely on the engineer to remember to push a tag for the previous state.
As part of the PR to main, we could link to the diff of the actual submodule, a bit like we do for the CRT.
| #### Sync with the origin | ||
| 1. Add a new remote if haven't done it already `git remote add fuser https://github.com/cberner/fuser.git` | ||
| 2. Fetch from the remote `git fetch fuser` | ||
| 3. List all the FUSER's tags `git tag -l | grep v`, it'll output all of the FUSER versions like `v0.15.1`. Select the version that you'd want to sync with (latest one by default) |
There was a problem hiding this comment.
Is this not going to show all of Mountpoint and Fuser's tags?
Maybe we want something like:
git ls-remote --tags fuser | grep v
(What I'm normally doing is actually having a separate repo for Fuser, is that what is assumed here?)
There was a problem hiding this comment.
This is still an issue.
There was a problem hiding this comment.
It suggests to locally list the local tags to make sure the target one is fetched so we can rebase locally as a next step.
Maybe I can expand a bit here.
| 3. Get the PR merged | ||
|
|
||
|
|
||
| #### PR to `fuser/fork` |
There was a problem hiding this comment.
Is this section covering additional changes to the Mountpoint fork? (vs changes coming from upstream fuser)
|
|
||
| Create a new branch from `fuser/fork`, commit changes and create a PR with `awslabs:fuser/fork` as the base branch. | ||
|
|
||
| The checks for this PR will be broken as they run against the `main` branch which has different structure (this is a point for improvement), but we still can have a consensus on what we're pulling in. |
There was a problem hiding this comment.
non-blocking: most tests seem to pass. May it be just a matter of fixing formatting?
There was a problem hiding this comment.
Do we want it?
Most likely will lead to rebase conflicts in the future during the upstream sync
README.md
Outdated
| Create a new branch from `fuser/fork`, commit changes and create a PR with `awslabs:fuser/fork` as the base branch. | ||
|
|
||
| The checks for this PR will be broken as they run against the `main` branch which has different structure (this is a point for improvement), but we still can have a consensus on what we're pulling in. | ||
| After PR is reviewed it needs to be closed (as will don't want to further modify the history via GitHub Merge) and the commit needs to be cherry-picked on top of `fuser/fork` branch `git cherry-pick -m 1 <commit-hash>` |
There was a problem hiding this comment.
I'm confused here: merge does not modify the history. What's the difference with cherry-pick?
There was a problem hiding this comment.
Commit created for PR sits in the different branch. Since we're not going to merge the PR we need to cherry-pick it on top of fork/fuser.
But if we ok to re-format the fork so all the checks are green, we can merge the PR.
Or should we agree on that we still going to merge it despite it's red?
| name = "fuser" | ||
| description = "Filesystem in Userspace (FUSE) for Rust" | ||
| name = "mountpoint-s3-fuser" | ||
| description = "A fork of filesystem in Userspace (FUSE) implementation for Rust for use in Mountpoint S3 only" |
There was a problem hiding this comment.
Maybe something like this:
| description = "A fork of filesystem in Userspace (FUSE) implementation for Rust for use in Mountpoint S3 only" | |
| description = "A fork of fuser - Filesystem in Userspace (FUSE) for Rust - only for use in Mountpoint for Amazon S3" |
| 2. Pull in the changes from upstream including submodules `git pull upstream main --recurse-submodules` and update the submodule `git submodule update --remote mountpoint-s3-fuser`. | ||
| > We can avoid adding `--recurse-submodules` argument all the time by telling git to update submodules on every pull `git config set submodule.recurse true` | ||
|
|
||
| 1. Since we changed the `fuser/fork` branch and `mountpoint-s3-fuser` refers to it submodule update should produce changes in `mountpoint-s3-fuser` index as well as in `Cargo.lock` if fork's version was changed. We need to commit these changes |
There was a problem hiding this comment.
This step may also require updating the dependency in Cargo.toml
ujinho
force-pushed
the
fuser/fork-crate
branch
from
d97b4ba to
6dcd733
Compare
| 7. Once `fuser/fork` is in the desired state and the tag is ready we can consume the change in the `main` branch | ||
| 1. `git checkout main` | ||
| 2. Pull in the changes from upstream including submodules `git pull upstream main --recurse-submodules` and update the submodule `git submodule update --remote mountpoint-s3-fuser`. | ||
| > We can avoid adding `--recurse-submodules` argument all the time by telling git to update submodules on every pull `git config set submodule.recurse true` |
There was a problem hiding this comment.
I'd drop mentioning the git config, only because this guide is already complex.
| #### Sync with the origin | ||
| 1. Add a new remote if haven't done it already `git remote add fuser https://github.com/cberner/fuser.git` | ||
| 2. Fetch from the remote `git fetch fuser` | ||
| 3. List all the FUSER's tags `git tag -l | grep v`, it'll output all of the FUSER versions like `v0.15.1`. Select the version that you'd want to sync with (latest one by default) |
There was a problem hiding this comment.
This is still an issue.
| #### Publishing the fork | ||
| We want to treat the fork as a standard crate inside our project and follow the existing [manual](https://github.com/awslabs/mountpoint-s3/blob/main/mountpoint-s3-client/PUBLISHING_CRATES.md) for publishing. |
There was a problem hiding this comment.
Can we really follow that guide as it is? I'm wondering what we're meant to do in the post-release steps, where we go and update the version. Should we go and do that?
There was a problem hiding this comment.
I think rebasing should produce a new version of mountpoint-s3-fuser. We need to consume it by updating the dependency in MP, e.g.:
fuser = { path = "../mountpoint-s3-fuser", version = "0.1.0", features = ["abi-7-28"] }
gets updated to
fuser = { path = "../mountpoint-s3-fuser", version = "0.2.0", features = ["abi-7-28"] }
This should be done via PR to awslabs/mounpoint-s3/main, in the same PR which updates the version of the submodule. This is where the MP tests are run to verify the rebase result.
I think we don't need PRs to awslabs/mounpoint-s3/fuser/fork and can just force push to it, relying on the PR to main to catch regressions.
There was a problem hiding this comment.
Ah, never mind, I guess the testing bit is already covered by the previous step. My point is that checks on the PR to the main branch make it ok to force push to fuser/fork.
| #### Publishing the fork | ||
| We want to treat the fork as a standard crate inside our project and follow the existing [manual](https://github.com/awslabs/mountpoint-s3/blob/main/mountpoint-s3-client/PUBLISHING_CRATES.md) for publishing. |
There was a problem hiding this comment.
nit: spacing
| #### Publishing the fork | |
| We want to treat the fork as a standard crate inside our project and follow the existing [manual](https://github.com/awslabs/mountpoint-s3/blob/main/mountpoint-s3-client/PUBLISHING_CRATES.md) for publishing. | |
| #### Publishing the fork | |
| We want to treat the fork as a standard crate inside our project and follow the existing [manual](https://github.com/awslabs/mountpoint-s3/blob/main/mountpoint-s3-client/PUBLISHING_CRATES.md) for publishing. |
|
|
||
| To pull in new changes from upstream, you should fetch changes from the original upstream and rebase them locally. | ||
|
|
||
| 1. Add a new remote if haven't done it already `git remote add fuser https://github.com/cberner/fuser.git` |
There was a problem hiding this comment.
Shall we have a section for prerequisites?
- You should have a remote setup for the fuser upstream repository.
- You should have Mountpoint's main repository configured as the
upstreamremote.
| open a pull request on the Mountpoint repository branching from the `fuser/fork` branch and use `fuser/fork` as the base branch when creating the pull request. | ||
| #### Sync with the origin | ||
|
|
||
| To pull in new changes from upstream, you should fetch changes from the original upstream and rebase them locally. |
There was a problem hiding this comment.
Maybe we should expand on why this is complex, and the high level steps we're taking?
| To pull in new changes from upstream, you should fetch changes from the original upstream and rebase them locally. | |
| To pull in new changes from upstream, we'll need to fetch the changes and rebase our patches on top, as well as maintain new versioning information. | |
| Since the published version of this crate `mountpoint-s3-fuser` uses Git submodules and we use rebased branches to keep track of patches, we'll need to tag each version we take a dependency on in `mountpoint-s3-fuser` to avoid Git's garbage collector leading to Mountpoint commits with no available version of this code. |
| 4. **Important** Next step implies history change, so at this point we need to make sure that we have a snapshot of the current state of `fuser/fork` branch. It must be a tag, e.g. `fuser-fork-<version>` containing the head commit. We assume we have it since we supposed to be following this runbook during the previous sync. | ||
| 1. Create a new branch from `fuser/fork`, `git rebase v0.15.1 -i` there and pick our commits which we'd like to put on top of the target tag (all of the commits by default) | ||
| 2. Once rebase is finished and all the possible conflicts (at least in `Cargo.toml`) are solved bump the fork's version and commit the change. Original version is not equal to fork's version, but we have to bump either major or patch part of it depending on what's changed in the original |
There was a problem hiding this comment.
I think we can tone down the explanation here if we cover it before the instructions. This can boil down to "we're going to now tag this commit so it can be referenced by submodules".
I think we can drop the assumption of the previous sync, it only adds complexity to the instructions here.
| 5. We might want to create a "read-only" PR from the rebased branch with `awslabs:fuser/fork` as the base branch. This PR should never get merged as it'll produce a new commit, but this is a good way for us to have a collaborative consensus on what we're pulling in. After the PR is reviewed it needs to be closed | ||
| > *Note* If we're sure what we're doing the PR step can be skipped and we can force push rebased branch into `fuser/fork` |
There was a problem hiding this comment.
Should we do this or not? Might is ambiguous.
I think we can cover this with the submodule PR tbh, and if we don't like it we can force push the tag maybe. (??? Maybe we shouldn't allow that at all.)
We can recommend here to open a draft PR and get it reviewed by a peer. If approved, then we can push a tag with a two-person review and we can also turn on tag protection rules for fuser?
There was a problem hiding this comment.
Ugh.. Maybe we just create a new repo for it?
I can't help thinking that we just make it even more cumbersome
Signed-off-by: Evgeny (Zhenia) Dolgii <[email protected]>
ujinho
force-pushed
the
fuser/fork-crate
branch
from
6dcd733 to
bcb8812
Compare
| 2. Once rebase is finished and all the possible conflicts (at least in `Cargo.toml`) are solved bump the fork's version and commit the change. Original version is not equal to fork's version, but we have to bump either major or patch part of it depending on what's changed in the original | ||
| 5. We might want to create a "read-only" PR from the rebased branch with `awslabs:fuser/fork` as the base branch. This PR should never get merged as it'll produce a new commit, but this is a good way for us to have a collaborative consensus on what we're pulling in. After the PR is reviewed it needs to be closed | ||
| > *Note* If we're sure what we're doing the PR step can be skipped and we can force push rebased branch into `fuser/fork` | ||
| 6. After PR is approved or changes pushed directly to `awslabs:fuser/fork` we need to create a tag for persisting the branch history. |
There was a problem hiding this comment.
after the fuser/fork was rebased and force-pushed, do the previous tags still stay valid?
e.g. if you're pushing fuser-fork-0.2.0 tag and MP repo still points to fuser-fork-0.1.0, will git be able to retrieve fuser-fork-0.1.0 with --recurse-submodules?
This is a "read only PR", not to be merged!
Rename fork of FUSER library to
mountpoint-s3-fuserfor publishingThis is a commit on top of the rebased
fuser/forkbranch.We've created a fuser-fork-0.15.0 tag, pulled in the changes from the original upstream and pushed the branch.
Next steps:
We will cherry-pick this commit into the
fuser/forkbranch and push it into remoteAfter that we will be able to cut a PR for the main package to consume the renamed fork
I will publish
mountpoint-s3-fuseron crates.io on 2/25. It was published already but I deleted it today for the sake of experiment and now it is a cooldown on name reuse for 1 day.Does this change impact existing behavior?
No, this is an aux change
Does this change need a changelog entry? Does it require a version change?
We set the version of the
mountpoint-s3-fuseras0.1.0since this is now considered a new crateBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and I agree to the terms of the Developer Certificate of Origin (DCO).