Skip to content

Enable running in subdirectories#76

Draft
pindab0ter wants to merge 10 commits into
ProjektGopher:mainfrom
pindab0ter:feature/subdirectory
Draft

Enable running in subdirectories#76
pindab0ter wants to merge 10 commits into
ProjektGopher:mainfrom
pindab0ter:feature/subdirectory

Conversation

@pindab0ter

@pindab0ter pindab0ter commented Aug 19, 2024

Copy link
Copy Markdown

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
Copy Markdown
Owner

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

@pindab0ter

Copy link
Copy Markdown
Author

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

@ProjektGopher

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Owner

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

@pindab0ter

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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