whisky icon indicating copy to clipboard operation
whisky copied to clipboard

Enable running in subdirectories

Open pindab0ter opened this issue 1 year ago • 12 comments

This is a proof of concept for being able to run hooks from a non-project-root directory within the git repository.

Resolves #75.

My testing setup is as follows:

  • cd .. && mkdir whisky-test && cd whisky-test (whisky-test and whisky are now next to eachother)
  • git init
  • mkdir subdirectory && cd subdirectory
  • composer install
  • Add this to composer.json:
        "repositories": [
          {
              "type": "path",
              "url": "../../whisky",
              "options": {
                  "symlink": true
              }
          }
      ],
    
  • composer update
  • vendor/bin/whisky install
  • Update whisky.json to have the pre-commit hooks contain "echo 'Running pre-commit hook'".

After making changes in the whisky source I needed to run composer build to update the symlinked binary so that I could test with it locally.

The biggest area that would need improvement that I can think of right now is what the story is for changing subdirectories. On install the subdirectory is now included in the pre-commit script as we need to change working directories (see Hook.php:134). Say the directory changes, there is no way or documentation on how to update Whisky's configuration. I suppose checking for existing lines in the pre-commit file containing vendor/bin/whisky and updating them would be the solution to this.

Please let me know if there's anything I missed or if you would like me to flesh out certain aspects.

pindab0ter avatar Aug 19 '24 11:08 pindab0ter

Thx a ton for this work! I'll hack on it this week and tag a new release

ProjektGopher avatar Aug 19 '24 18:08 ProjektGopher

I also can't think of any reason why this wouldn't be backwards compatible.

pindab0ter avatar Aug 19 '24 20:08 pindab0ter

I also can't think of any reason why this wouldn't be backwards compatible.

Only thing that jumped out at me is that the new snippet replaces the old one, instead of moving the old one to the list of legacy snippets

ProjektGopher avatar Aug 19 '24 20:08 ProjektGopher

Ah right, I wasn't familiar with that flow yet. I also don't know if pushd/popd is the best solution. If anything it's on the safe side, so that might be good.

pindab0ter avatar Aug 19 '24 21:08 pindab0ter

I've actually never even heard of pushd/popd I'm gonna have to look those up, haha

ProjektGopher avatar Aug 19 '24 22:08 ProjektGopher

They allow you to push a directory on the stack and then pop it off again, returning you to were you were before. This is a reliable way of navigating to and from directories. The added benefit over a simple cd is that cd won't remember where you came from.

pindab0ter avatar Aug 19 '24 23:08 pindab0ter

I think it works the same as "cd [folder]" and "cd -"... but it seems pushd and popd are supported on windows

gpibarra avatar Aug 19 '24 23:08 gpibarra

I think I've got most of our bugs squashed, and the test suite is passing again, but I haven't really set up a test for this specific use case, so let me see if I've got this right:

We have a directory that's a git repo, no composer installed, but a series of subdirectories. Each subdirectory could now be a composer project. Do we want to install whisky in only one of these subdirectories? Do we want all subdirectories to run a common set of hooks? Do we then have the whisky.json file in the root of the git repo, instead of the root of each composer project? If it's in the git project root, might it make more sense to use a global whisky install?

example:

mkdir mono_repo && cd mono_repo && git init
mkdir sub1 && cd sub1 && composer init && composer require projektgopher/whisky --dev && vendor/bin/whisky install
git hook run pre-commit # this should execute just fine
cd .. && git hook run pre-commit # I think currently this will break
mkdir sub2 && cd sub2 && composer init
git hook run pre-commit # this will almost certainly fail

I think that before we go any further we've got to really talk through our use story.

Drafting for now.

ProjektGopher avatar Sep 04 '24 21:09 ProjektGopher

Do we want to install whisky in only one of these subdirectories?

This is our use case, yes. Our repo contains a directory for deployment/infrastructure related files and besides that the directory for the Laravel project.

I suggest finishing the PR as it is now. It adds support for the use case where the single repo is not in the root directory without adding much complexity at all or changing anything for existing users.

Support for multiple projects in the same repo should be its own PR imho.

pindab0ter avatar Sep 05 '24 06:09 pindab0ter

I think the reason I'm holding off on this is that a real-world use I can see is git worktrees. If someone is using that feature in order to have multiple branches checked out at once (super useful for teams) this would need to be compatible with that.

ProjektGopher avatar Sep 09 '24 18:09 ProjektGopher

What incompatibilities do you foresee? I'm not a regular worktree user, so I would love to know how this PR interacts with that.

pindab0ter avatar Sep 09 '24 18:09 pindab0ter

So a work tree allows you to install the repo into an empty folder like using the bare flag. Then in your work trees directory each checked out branch has its own directory. I can then switch branches without having to stash or commit. This means that multiple branches are checked out at once. If I try to run my hooks it wouldn't know which directory to look in for the whisky config, and whichever directory is chosen, those hooks would run unchanged in all branches

ProjektGopher avatar Sep 09 '24 22:09 ProjektGopher