Skip to content
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

Enable running in subdirectories #76

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

pindab0ter
Copy link

@pindab0ter pindab0ter commented Aug 19, 2024

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.

@ProjektGopher
Copy link
Owner

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

@pindab0ter
Copy link
Author

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

@ProjektGopher
Copy link
Owner

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

@pindab0ter
Copy link
Author

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.

@ProjektGopher
Copy link
Owner

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

@pindab0ter
Copy link
Author

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.

@gpibarra
Copy link
Contributor

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

@ProjektGopher
Copy link
Owner

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 ProjektGopher marked this pull request as draft September 4, 2024 21:12
@pindab0ter
Copy link
Author

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.

@ProjektGopher
Copy link
Owner

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.

@pindab0ter
Copy link
Author

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

@ProjektGopher
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to install Whisky in subdirectory
3 participants