Skip to content

Loading…

Fixing Munge of ENV #54

Merged
merged 1 commit into from

6 participants

@jkamenik

I had a problem where all the builds started to fail except the first after a CIJoe restart. I traced the problem to the munging of the ENV when a hook is called. I have slightly modified the run_hook function to provide a clean ENV to the hook file, but to reinstate the ENV for the next build.

@nt
nt commented

Hello @jkamenik,

I am currently runnin into some trouble with my CI Joe setup. It looks like the same bug you encountered. Could you confirm please ?

http://stackoverflow.com/questions/6030840/rails-dependency-problem-within-ci-joe

Thank you,

Nicolas.

@jkamenik

Your setup seems to be similar to mine. I was using RVM, but because the ENV pollution it would not stay clean CIJoe would only checkout and build 1 time then it would crash. My ci command was rvm use 1.8.7@gemset; bundle install; rake spec. After the first build the gem set would be wrong and cijoe could not handle it. Cleaning the ENV prevents ENV changes from effecting CIJoe.

@queso
Collaborator

hey guys,

Thanks for sending in the pull request, I will try to get this tested and merged in the next day or so.

@nt
nt commented

Thanks @jkamenik. I tried your patch, it worked like a breeze. I'm up and running :)
Cheers from France.

@coverall

Ah, this is just what I need! Hope this gets merged in :)

@jacqui

Yes please!

@dblock

+1 fixes it for me

@queso queso merged commit 7c7ea5d into defunkt:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Showing with 87 additions and 1 deletion.
  1. +6 −1 lib/cijoe.rb
  2. +81 −0 test/test_hooks.rb
View
7 lib/cijoe.rb
@@ -189,9 +189,14 @@ def run_hook(hook)
{}
end
+ orig_ENV = ENV.to_hash
ENV.clear
data.each{ |k, v| ENV[k] = v }
- `cd #{@project_path} && sh #{file}`
+ output = `cd #{@project_path} && sh #{file}`
+
+ ENV.clear
+ orig_ENV.to_hash.each{ |k, v| ENV[k] = v}
+ output
end
end
View
81 test/test_hooks.rb
@@ -0,0 +1,81 @@
+require 'helper'
+
+# mock files to true
+class File
+ class << self
+ alias orig_exists? exists?
+ alias orig_executable? executable?
+
+ def exists?(f)
+ return true if $hook_override
+ orig_exists? f
+ end
+ def executable?(f)
+ return true if $hook_override
+ orig_executable? f
+ end
+ end
+end
+
+# #mock file to be the file I want
+class CIJoe
+ attr_writer :last_build
+ alias orig_path_in_project path_in_project
+ alias orig_git_user_and_project git_user_and_project
+
+ def path_in_project(f)
+ return '/tmp/test' if $hook_override
+ orig_path_in_project
+ end
+
+ def git_user_and_project
+ return ['mine','yours'] if $hook_override
+ orig_git_user_and_project
+ end
+end
+
+class CIJoe::Commit
+ attr_writer :raw_commit
+end
+
+
+
+class TestHooks < Test::Unit::TestCase
+ def teardown
+ $hook_override = nil
+ end
+
+ def setup
+ $hook_override = true
+ open("/tmp/test",'w') do |file|
+ file.write "echo $test\n"
+ file.write "echo $AUTHOR\n"
+ file.write "export test=foo\n"
+ end
+ File.chmod(0777,'/tmp/test')
+
+ @cijoe = CIJoe.new('/tmp')
+ @cijoe.last_build = CIJoe::Build.new "path", "user", "project", Time.now, Time.now,
+ "deadbeef", :failed, "output", nil
+ @cijoe.last_build.commit.raw_commit = "Author: commit author\nDate: now"
+ end
+
+ def test_leave_env_intact
+ ENV['AUTHOR'] = 'foo'
+ @cijoe.run_hook("/tmp/test")
+ assert ENV.size != 0, 'ENV is empty but should not be'
+ assert ENV['AUTHOR'] == 'foo', 'ENV munged a value'
+ end
+
+ def test_empty_hook_env
+ ENV["test"] = 'should not be shown'
+ output = @cijoe.run_hook("/tmp/test")
+ assert_equal "\ncommit author\n", output
+ end
+
+ def test_hook_munge_env
+ ENV['test'] = 'bar'
+ output = @cijoe.run_hook("/tmp/test")
+ assert ENV['test'] == 'bar'
+ end
+end
Something went wrong with that request. Please try again.