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

Restore hooks PATH before calling ruby. #186

Merged
merged 1 commit into from Nov 18, 2014

Conversation

6 participants
@cirosantilli
Copy link
Contributor

commented Oct 18, 2014

@Razer6

This comment has been minimized.

Copy link
Member

commented Oct 30, 2014

@randx Can you take a look? This is a starting point to fix errors if users have multiple rubies on the computer. This issue is popping of regularly in the last week. /cc @dosire

@dzaporozhets

This comment has been minimized.

Copy link
Member

commented Oct 30, 2014

Can you please explain what kind of problem with multiple rubies we have and how it fixes it?

@Razer6

This comment has been minimized.

@cirosantilli

This comment has been minimized.

Copy link
Contributor Author

commented Oct 30, 2014

@randx this exists solely for: gitlabhq/gitlabhq#8131, rationale is there.

@dzaporozhets

This comment has been minimized.

Copy link
Member

commented Nov 17, 2014

@cirosantilli can you please do next?

  • take a look if this code compatible with custom hook support? (#190)
  • make it mergeable
  • bump version to 2.3.0
  • add CHANGELOG item
@vsizov

This comment has been minimized.

Copy link
Contributor

commented Nov 17, 2014

looks good for me

@cirosantilli cirosantilli force-pushed the cirosantilli:restore-path branch from dc93b4a to cd9e796 Nov 17, 2014

@cirosantilli

This comment has been minimized.

Copy link
Contributor Author

commented Nov 17, 2014

Updated. I think it works with the custom hooks.

dzaporozhets added a commit that referenced this pull request Nov 18, 2014

Merge pull request #186 from cirosantilli/restore-path
Restore hooks PATH before calling ruby.

@dzaporozhets dzaporozhets merged commit 53fed51 into gitlabhq:master Nov 18, 2014

1 check passed

default The build passed on Semaphore.
Details

@cirosantilli cirosantilli deleted the cirosantilli:restore-path branch Nov 18, 2014

@zzjin

This comment has been minimized.

Copy link

commented on cd9e796 Nov 18, 2014

$GIT_DIR where to store?
when update to this master gitlab fail with push. show remote: ruby: No such file or directory -- ./hooks/pre-receive-main.rb (LoadError)

which means $GIT_DIR not found or replaced with .
need to setup gir_dir in global PATH?

This comment has been minimized.

Copy link
Contributor Author

replied Nov 18, 2014

I had understood that GIT_DIR was set automatically (sometimes to ., but that in this case the current directory would be set properly to match it)

But maybe I got it wrong:

http://stackoverflow.com/questions/7065224/in-a-git-hook-is-the-current-working-directory-guaranteed-to-be-within-the-git-r

http://longair.net/blog/2011/04/09/missing-git-hooks-documentation/

I will try to figure it out.

This comment has been minimized.

Copy link
Contributor Author

replied Nov 18, 2014

Do you get the error on every push? I didn't get and error by:

  • creating a new repo
  • pushing to it

This comment has been minimized.

Copy link
Contributor Author

replied Nov 18, 2014

I have just tested and what I though seems to be correct: GIT_DIR is set to ., but the pwd is set correctly such that this it should point to the ruby file. Also try printing your pwd. Of course, I may have done something else wrong, this is a super hard PR. Let me know if you find something.

This comment has been minimized.

Copy link

replied Nov 18, 2014

Command pwd shows /home/git/gitlab-shell/hooks in post-receive file. Can we just use ruby "$GIT_DIR/pre-receive-main.rb" to run ruby shell, since all used three pre-receive&update&post-receive set GIT_DIR to .?

This comment has been minimized.

Copy link
Contributor Author

replied Nov 18, 2014

Hmmm this is weird... when I run tests on minimal git repos pwd points to the .git, not the hooks. Git 1.9.1 here. Can you try that as well? Running a minimalistic non bare -> bare push and setting the hook files to on the bare to:

#!/bin/sh

echo 'pre-receive'
pwd
echo "$GIT_DIR"

This comment has been minimized.

Copy link

replied Nov 19, 2014

Same for me... breaks whole git push:

remote: hooks/pre-receive: line 5: ruby: No such file or directory

If I use

echo "$GIT_DIR"

I get: remote: .

Why restoring PATH before calling hooks? Whats the benefit? 2.2.0 works just fine.
I'am using Git 1.9.0

Update://
Problem is, that the 'ruby' command isn't available.
It seems, that it doesn't load the .bashrc nor the bash_profile.
I have downgraded to Gitlab Shell 2.2.0.

This comment has been minimized.

Copy link
Contributor Author

replied Nov 19, 2014

Why restoring PATH before calling hooks? Whats the benefit? 2.2.0 works just fine.

See #186

Problem is, that the 'ruby' command isn't available.

Weird, because the path is being set at: https://github.com/gitlabhq/gitlabhq/blob/8e7fa0c2a1add0c583409c2617bc06e949cd3a72/config/application.rb#L97 , you have updated gitlab as well right?

I propose that we move the discussion to the PR: #186

@zzjin

This comment has been minimized.

Copy link

commented Nov 19, 2014

right with lastest gitlab master install on same machine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.