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

Make pushing changes from Git dependency repositories easier #2226

Merged
merged 4 commits into from Apr 25, 2017

Conversation

Projects
None yet
2 participants
@tomasdeml
Contributor

tomasdeml commented Apr 11, 2017

This PR mostly addresses issues raised in #2119 and changes the way Git dependencies are initialized to make pushing changes from the cloned repository easier. The following workflow will be now possible/easier with Paket:

  1. Add a git dependency to paket.dependencies, then run paket update/install to clone and checkout the repo.
  2. Checkout master or create a feature branch in the cloned (downstream) repo, i.e. the repo under paket-files directory.
  3. Make changes in the cloned repo, run tests and commit the changes.
  4. Push the changes from the cloned repo to the origin (upstream) repo, i.e. the repo specified in paket.dependencies.
  5. Run paket update to update paket.lock with SHA1 of the changes pushed to the upstream repo.

I understand this PR may be controversial as it develops Paket in a way that may be currently served by Git submodules. However, Paket has some advantages over submodules:

  • Paket supports semver tags when resolving Git dependencies
  • Paket caches the upstream repository locally which helps build servers and saves space/time when working with multiple projects depending on same Git dependency

The changes helped our company to move away from (very painful) Nuget source packages. Feel free to suggest changes or reject the PR entirely :-). However, if you choose to accept the proposed changes, a migration strategy has to be devised before merging the PR. Otherwise users would be required to clear Paket local temp directory and respective paket-files directories as this is a breaking change.

The following changes were made to the implementation of Git dependencies:

  • Cache repository (i.e. the repo under %USERPROFILE%\.paket\git\db) is cloned as a bare mirror

    Bare clone saves disk space as it does not need a working directory. Mirror clone tracks the upstream remote more closely as it always recreates refs.

  • Downstream repository is configured with the origin remote pointing to the upstream repository (instead of the cache repository)

    Having the upstream repo configured as the origin makes pushing changes to the upstream more natural. We also want to prevent user from pushing the changes to the cache directly - the cache should be managed by Paket.

  • Tracking branch for each downstream repo (b<REPO_PATH_HASH>) is no longer created in the cache repo

    The branch IMO serves no practical purpose now, see next point.

  • paket/lock tag is created and checked out in the downstream repository when updateing/restoreing the dependency

    This allows user to see what the original checked out commit was when making changes. By checking out the tag - instead of resetting the downstream repo --hard to the tracking branch - the repo starts in the detached HEAD state which:

    • allows easy experimentation - user can discard her commits simply by checking out the paket/lock tag again
    • forces user to checkout a branch when doing meaningful changes.
  • Downstream repository is checked for uncommitted changes before updateing/restoreing the dependency

    This prevents hard-to-track bugs as well as non-repeatable builds that might occur by merging user's uncommitted changes into the new working directory.

  • Downstream repository is checked for commits made in the detached HEAD state before updateing/restoreing the dependency

    This makes working in the downstream repository safer as it saves user from having to inspect reflog after "losing" her commits. It also forces the user to move the changes to a permanent branch when making meaningful changes.

No tests were impacted by the changes. However, there are currently few integration tests covering Git dependencies.

tomasdeml added some commits Apr 7, 2017

Cache repository for Git dependency is cloned as a bare mirror
Bare clone saves disk space as it does not need a working directory. Mirror clone tracks the upstream remote more closely as it always recreates refs.
`paket/lock` tag is created and checked out in the downstream reposit…
…ory when `update`ing/`restore`ing the dependency

This allows user to see what the original checked out commit was when making changes.
By checking out the tag - instead of resetting the downstream repo `--hard` to the tracking branch - the repo starts in the detached HEAD state which:
- allows easy experimentation - user can discard her commits simply by checking out the `paket/lock` tag again
- forces user to checkout a branch when doing meaningful changes.
Note that the tracking branch for each downstream repo (`b<REPO_PATH_HASH>`) is no longer created.
Downstream repository is configured with the `origin` remote pointing…
… to the upstream repository (instead of the cache repository)

Having the upstream repo configured as the `origin` makes pushing changes to the upstream more natural.
We also want to prevent user from pushing the changes to the cache directly - the cache should be managed by Paket.
Note that fetching from the cache has been updated to use explicit refspec. This way we do not need an entry with matching URL in gitconfig.
Downstream repository is checked for uncommitted changes and commits …
…made in detached HEAD state when `update`ing/`restore`ing Git dependency

Checking for uncommitted changes prevents hard-to-track bugs as well as non-repeatable builds that might occur by merging user's changes into the new working directory.
Checking for detached commits makes working in the downstream repository safer - it saves the user from having to inspect reflog after "losing" her commits.
It also forces the user to move the changes to a permanent branch when making meaningful changes.
@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Apr 12, 2017

Member

/cc @Krzysztof-Cieslak you are one of the git deps users - can you please take a look?

Member

forki commented Apr 12, 2017

/cc @Krzysztof-Cieslak you are one of the git deps users - can you please take a look?

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Apr 24, 2017

Member

is this ready?

Member

forki commented Apr 24, 2017

is this ready?

@forki forki merged commit 117cad1 into fsprojects:master Apr 25, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@tomasdeml

This comment has been minimized.

Show comment
Hide comment
@tomasdeml

tomasdeml Apr 25, 2017

Contributor

Oh, I did not expect the PR to be merged so easily 😄. Just to be sure - so the missing migration strategy is not an issue for this breaking change? Maybe the major version number should be incremented at the very least?

Contributor

tomasdeml commented Apr 25, 2017

Oh, I did not expect the PR to be merged so easily 😄. Just to be sure - so the missing migration strategy is not an issue for this breaking change? Maybe the major version number should be incremented at the very least?

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Apr 25, 2017

Member

Can you please send a pull request to the release notes that makes this manual step clear?

Member

forki commented Apr 25, 2017

Can you please send a pull request to the release notes that makes this manual step clear?

@tomasdeml

This comment has been minimized.

Show comment
Hide comment
@tomasdeml

tomasdeml Apr 25, 2017

Contributor

Sure, hang on...

Contributor

tomasdeml commented Apr 25, 2017

Sure, hang on...

@tomasdeml tomasdeml deleted the tomasdeml:git-dependency-enh branch Apr 25, 2017

@tomasdeml tomasdeml changed the title from [WIP] Make pushing changes from Git dependency repositories easier to Make pushing changes from Git dependency repositories easier Apr 25, 2017

tomasdeml added a commit to tomasdeml/Paket that referenced this pull request Apr 25, 2017

forki added a commit that referenced this pull request Apr 25, 2017

Merge pull request #2278 from tomasdeml/pr-2226-release-notes
Document migration steps required for PR #2226
@tomasdeml

This comment has been minimized.

Show comment
Hide comment
@tomasdeml

tomasdeml May 17, 2017

Contributor

Just wanted to share that we have stumbled upon an issue when using Git dependencies with Git LFS. Git LFS cannot fetch files using the file protocol. As this protocol is used when fetching from local Paket cache repository, a workaround must be applied. Before invoking paket install/restore/update, you have to:

  1. Disable Git LFS smudge filter via an env var
  2. Redirect Git LFS endpoint in local config of the downstream repository to the remote endpoint of the original upstream repository.

Once the install/restore/update command completes, you have to explicitly pull large files from the new LFS endpoint (git lfs pull).

Contributor

tomasdeml commented May 17, 2017

Just wanted to share that we have stumbled upon an issue when using Git dependencies with Git LFS. Git LFS cannot fetch files using the file protocol. As this protocol is used when fetching from local Paket cache repository, a workaround must be applied. Before invoking paket install/restore/update, you have to:

  1. Disable Git LFS smudge filter via an env var
  2. Redirect Git LFS endpoint in local config of the downstream repository to the remote endpoint of the original upstream repository.

Once the install/restore/update command completes, you have to explicitly pull large files from the new LFS endpoint (git lfs pull).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment