From: jiil.hyoun <jiil.hyoun@samsung.com>
Date: Thu, 23 Aug 2012 12:12:38 +0000 (+0900)
Subject: [Title] apply nitpick for all source code
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=7aabc0ef322ba6311e05ac5aab4f668e73f24575;p=sdk%2Ftools%2Fsdk-build.git

[Title] apply nitpick for all source code
[Type] Enhancement
[Module] Toolchain /
[Priority] Major
[Jira#]
[Redmine#]
[Problem]
[Cause]
[Solution]
[TestCase]

Change-Id: Ibeddb49ff51bcadf1831a315399a562b45136760
---

diff --git a/pkg-svr b/pkg-svr
index 6698a94..3e3d597 100755
--- a/pkg-svr
+++ b/pkg-svr
@@ -99,7 +99,7 @@ begin
 
 		server.remove_dist( option[:dist] )
 	when "remove-pkg"
-		server.remove_pkg( option[:dist], option[:pkgs], option[:os], option[:recursive] )
+		server.remove_pkg( option[:dist], option[:pkgs], option[:os] )
 	when "remove-snapshot"
 		server.remove_snapshot( option[:dist], option[:snaps] )
 	when "clean"
diff --git a/src/build_server/BuildComm.rb b/src/build_server/BuildComm.rb
index 4b8e02b..be741ce 100644
--- a/src/build_server/BuildComm.rb
+++ b/src/build_server/BuildComm.rb
@@ -68,7 +68,7 @@ class BuildCommServer
 			req = @tcp_server.accept		
 	
 			begin
-				yield req
+				yield req if block_given?
 			rescue
 				@log.error $!
 				@log.error "Caught a connection exception"
@@ -194,34 +194,32 @@ class BuildCommServer
 		#    After downloading it, send "SUCC"
 		begin
             req.puts "READY"
-            while l = req.gets()
-                tok = l.split(",").map { |x| x.strip }
-                cmd = tok[0].strip
-                if cmd == "UPLOADED" then
-                	@log.info "Client uploaded file to ftp server successful"
-                    if tok.count < 6 then
-                        @log.error "Server received wrong REQ : #{l.strip}"
-                        req.puts "ERROR"
-                        return false
-                    end
-                    filepath = tok[3].strip
-                    dirname = File.dirname(filepath)
-                    filename = File.basename(filepath)
+			while l = req.gets()
+				tok = l.split(",").map { |x| x.strip }
+				cmd = tok[0].strip
+				if cmd == "UPLOADED" then
+					@log.info "Client uploaded file to ftp server successful"
+					if tok.count < 6 then
+						@log.error "Server received wrong REQ : #{l.strip}"
+						req.puts "ERROR"
+						return false
+					end
+					filepath = tok[3].strip
 
 					# get ftp connection info
-                    if @ftp_url.nil? then
-                    	ip = tok[1].strip
-                    	port = tok[2].strip
-                    	username = tok[4].strip
-                    	passwd = tok[5].strip
-                       	@log.info "Client sent ftp server infomations [#{ip}, #{port}]"
-                    else
-                    	url_contents = Utils.parse_ftpserver_url(@ftp_url)
-                        ip = url_contents[0]
-                        port = url_contents[1]
-                    	username = url_contents[2]
-                    	passwd = url_contents[3]
-                    end
+					if @ftp_url.nil? then
+						ip = tok[1].strip
+						port = tok[2].strip
+						username = tok[4].strip
+						passwd = tok[5].strip
+						@log.info "Client sent ftp server infomations [#{ip}, #{port}]"
+					else
+						url_contents = Utils.parse_ftpserver_url(@ftp_url)
+						ip = url_contents[0]
+						port = url_contents[1]
+						username = url_contents[2]
+						passwd = url_contents[3]
+					end
 
 					# download from ftp server
 					dst_filepath = nil
@@ -380,7 +378,7 @@ class BuildCommClient
 				if line.strip == "=CHK" then next end
 
 				# print
-				yield line.strip
+				yield line.strip if block_given?
 			end
 		rescue Timeout::Error
             puts "WARN: Connection timed out"
diff --git a/src/build_server/BuildJob.rb b/src/build_server/BuildJob.rb
index d8b636e..51b52bd 100644
--- a/src/build_server/BuildJob.rb
+++ b/src/build_server/BuildJob.rb
@@ -116,7 +116,7 @@ class BuildJob
 
 
 	def is_sub_job?
-		return (not @parent.nil?) ? true : false
+		return (not @parent.nil?)
 	end
 
 
@@ -318,7 +318,6 @@ class BuildJob
 		end
 	
 		# check compat os	
-		compatable_packages = {} # | packag, os_list |
 		@pkginfo.get_target_packages(@os).each do |p|
 			if not p.os_list.include?(o.os) then return false end
 		end
@@ -958,7 +957,7 @@ class BuildJob
 		for pkg in @pending_ancestor.pkginfo.get_target_packages(@os)
 			local_pkgs.push "#{src_path}/#{pkg.package_name}_#{ver}_#{@os}.zip"
 		end	
-		for job in @pending_ancestor.rev_success_jobs
+		@pending_ancestor.rev_success_jobs.each do |job|
 			src_path = job.source_path
 			ver = job.pkginfo.get_version()
 			for pkg in job.pkginfo.get_target_packages(@os)
@@ -980,7 +979,7 @@ class BuildJob
 
 		# check reverse dependecy and update parent rev_fail_project list
 		new_fail_projects = ReverseBuildChecker.check(self, false)
-		for p in new_fail_projects
+		new_fail_projects.each do |p|
 			@pending_ancestor.add_rev_fail_project(p[0], p[1])
 		end
 
@@ -1006,7 +1005,7 @@ class BuildJob
 
 				sleep 1
 			end
-        end 
+        end
 
 		return true
 	end
@@ -1017,7 +1016,6 @@ class BuildJob
 	
 		# get package path list
 		binpkg_path_list = Dir.glob("#{@source_path}/*_*_#{@os}.zip")
-		srcpkg_path_list = Dir.glob("#{@source_path}/*.tar.gz")
 
 		# upload
 		u_client = Client.new( @server.pkgserver_url, nil, @log )
diff --git a/src/pkg_server/client.rb b/src/pkg_server/client.rb
index 75a0480..7e62827 100644
--- a/src/pkg_server/client.rb
+++ b/src/pkg_server/client.rb
@@ -277,7 +277,7 @@ class Client
 	end	
 
 	private
-	def get_cached_filepath(pkg_filename, pkg_checksum, pkg_size, os)
+	def get_cached_filepath(pkg_filename, pkg_checksum, pkg_size)
 
 		cached_filepath = "#{@download_path}/#{pkg_filename}"
 		if File.exist? cached_filepath then
@@ -294,9 +294,6 @@ class Client
     # download dependent source
     def download_dep_source(file_name)
 
-        file_url = nil
-
-        addr_arr = @server_addr.split('/')
         file_url = @server_addr + "/source/#{file_name}"
         if not FileDownLoader.download(file_url, @location, @log) then
             @log.error "Failed download #{file_name}"
@@ -476,7 +473,6 @@ class Client
         end
 
         dist = ""
-        server_arr = server.split("/")
 		dist = File.basename(server)
 
         return dist
@@ -596,9 +592,7 @@ class Client
             return false
         end
         pkg_name = filename.split("_")[0]
-        type = "binary"
         manifest_file = "pkginfo.manifest"
-        pkg_config_path = File.join(@location, PACKAGE_INFO_DIR, pkg_name)
 
         uniq_name = Utils.create_uniq_name
         path = Utils::HOME + "/tmp/#{uniq_name}"
@@ -624,18 +618,18 @@ class Client
 			return false
         end
 
-       	compare_result = compare_version_with_installed_pkg(pkg_name, new_pkg_ver)
-       	if not force then
-           	case compare_result
-           	when -1 then
-               	@log.warn "installed \"#{pkg_name}\" package version is bigger.."
-               	return true
-           	when 0 then
-               	@log.warn "\"#{pkg_name}\" package version is same with installed package version.."
-               	return true
-           	when 1, 2 then
-           	end
-       	end
+		compare_result = compare_version_with_installed_pkg(pkg_name, new_pkg_ver)
+		if not force then
+			case compare_result
+			when -1 then
+				@log.warn "installed \"#{pkg_name}\" package version is bigger.."
+				return true
+			when 0 then
+				@log.warn "\"#{pkg_name}\" package version is same with installed package version.."
+				return true
+			when 1, 2 then
+			end
+		end
 
        	if check_installed_pkg(pkg_name) then
            	uninstall(pkg_name, false)
@@ -644,26 +638,31 @@ class Client
 		if trace then
 			install_dep_pkgs = pkg.install_dep_list
 			new_pkg_os = pkg.os
-			install_dep_pkgs.each do |pkg|
+			install_dep_pkgs.each do |p|
 				# check local path first
-				binpkgs = Dir.glob("#{repos_path}/#{pkg.package_name}_*_#{new_pkg_os}.zip")
+				binpkgs = Dir.glob("#{repos_path}/#{p.package_name}_*_#{new_pkg_os}.zip")
 				if not binpkgs.empty? then
 					if not install_local_pkg_internal(binpkgs[0], true, false, repos_path) then
-						@log.warn "#{pkg} package is not installed"						
+						@log.warn "#{p} package is not installed"
 					end
 				else
-					if not install_internal(pkg.package_name, new_pkg_os, true, false) then
-						@log.warn "#{pkg} package is not installed"						
+					if not install_internal(p.package_name, new_pkg_os, true, false) then
+						@log.warn "#{p} package is not installed"
 					end
-				end						
+				end
 			end
-		end				
+		end
 
-       	# install package
-       	ret = FileInstaller.install(pkg_name, pkg_path, "binary", @location, @log)
+		# install package
+		ret = FileInstaller.install(pkg_name, pkg_path, "binary", @location, @log)
 
-       	add_local_pkg_info(pkg_name)
-       	write_pkg_hash_to_file(nil)
+		if not ret then
+			@log.error "Install failed \"#{pkg_path} [#{new_pkg_ver}]\" file.. "
+			return false
+		end
+
+		add_local_pkg_info(pkg_name)
+		write_pkg_hash_to_file(nil)
 
         @log.info "Installed \"#{pkg_path} [#{new_pkg_ver}]\" file.. OK"
         return true
@@ -1343,7 +1342,7 @@ class Client
         # install package
 		cached_filepath = nil
 		if Utils.is_linux_like_os( Utils::HOST_OS ) then
-			cached_filepath = get_cached_filepath(filename, pkg_checksum, pkg_size, os)
+			cached_filepath = get_cached_filepath(filename, pkg_checksum, pkg_size)
 		end			
 		if not cached_filepath.nil? then
 			@log.info "\"#{pkg_name}\" will be installed using cached file"				
diff --git a/src/pkg_server/distribution.rb b/src/pkg_server/distribution.rb
index 7ce2dae..9368a17 100644
--- a/src/pkg_server/distribution.rb
+++ b/src/pkg_server/distribution.rb
@@ -219,7 +219,7 @@ class Distribution
 		end
 		update_pkg_list = []
 
-        for os in @support_os_list 
+		@support_os_list.each do |os|
 			# error check 
 			if client.pkg_hash_os[os].nil? then 
 				@log.error("package server does not have os : #{os}", Log::LV_USER)
@@ -229,7 +229,7 @@ class Distribution
 			server_pkg_name_list = client.pkg_hash_os[os].keys
 			local_pkg_name_list = @pkg_hash_os[os].keys 
 			full_pkg_name_list = server_pkg_name_list + local_pkg_name_list
-            
+
 			full_pkg_name_list.each do |pkg_name| 
 				ret = sync_package( pkg_name, client, os, force )
 				if not ret.nil? then 
@@ -351,7 +351,7 @@ class Distribution
 		write_archive_pkg_list
 
 		# collect remaning file's name from snapshot list
-		for snapshot in remain_snapshot_list  
+		remain_snapshot_list.each do |snapshot|
 			os_info = "#{@location}/snapshots/#{snapshot}/#{OS_INFO_FILE}"
 			os_list = []
 			# if snapshot has os_info file then using that file
@@ -485,7 +485,7 @@ class Distribution
 		end
 	end 
 
-	def remove_pkg( pkg_name_list, os, recursive )  
+	def remove_pkg( pkg_name_list, os )  
 		if os.eql? "all" then os_list = @support_os_list
 		else os_list = [ os ]
 		end
@@ -714,11 +714,11 @@ class Distribution
 				return nil
 			end 
 				
-			pkg = sync_package2( server_pkg, client, os, force )
+			pkg = sync_package2( server_pkg, client, os )
 			return ["ADD", os, pkg]
 		# if package exist only server
 		elsif ( not server_pkg.nil? ) then
-			pkg = sync_package2( server_pkg, client, os, force )
+			pkg = sync_package2( server_pkg, client, os )
 			return ["ADD", os, pkg]
 		# if package exist only local
 		elsif ( not local_pkg.nil? ) then  
@@ -736,7 +736,7 @@ class Distribution
 		return nil
 	end 
 
-	def sync_package2( pkg, client, os, force ) 
+	def sync_package2( pkg, client, os )
 		pkg_name = pkg.package_name
 
 		# package update
diff --git a/src/pkg_server/packageServer.rb b/src/pkg_server/packageServer.rb
index a82abdb..ba7d8b9 100644
--- a/src/pkg_server/packageServer.rb
+++ b/src/pkg_server/packageServer.rb
@@ -407,13 +407,13 @@ class PackageServer
 		Utils.file_unlock(@lock_file)
 	end
 
-	def remove_pkg( dist_name, pkg_name_list, os, recursive )
+	def remove_pkg( dist_name, pkg_name_list, os )
 		@log.info "package remove in server"
 		distribution = get_distribution( dist_name )
 
 		@lock_file = Utils.file_lock(LOCK_FILE)
 		
-		distribution.remove_pkg(pkg_name_list, os, recursive)
+		distribution.remove_pkg(pkg_name_list, os)
 	
 		# generate snapshot	
 		@log.info "generaging snapshot"
diff --git a/test/build-cli-26.testcase b/test/build-cli-26.testcase
index 4d04bd2..b3c0684 100644
--- a/test/build-cli-26.testcase
+++ b/test/build-cli-26.testcase
@@ -12,15 +12,15 @@ cd git01;tar xf c_v4.tar.gz
 Info: Initializing job...
 Info: Invoking a thread for MULTI-BUILD Job
 Info: New Job
-Info: Added new job "testd" for ubuntu-32!
-Info: Added new job "testb" for ubuntu-32!
-Info: Added new job "testb" for windows-32!
-Info: Added new job "testa" for ubuntu-32!
-Info: Added new job "testa" for windows-32!
-Info: Added new job "testa1" for ubuntu-32!
-Info: Added new job "testa1" for windows-32!
-Info: Added new job "testc" for ubuntu-32! 
-Info: Added new job "testc" for windows-32! 
+Info: Added new job 
+Info: Added new job 
+Info: Added new job 
+Info: Added new job 
+Info: Added new job 
+Info: Added new job 
+Info: Added new job 
+Info: Added new job 
+Info: Added new job 
 Info:  * Sub-Job
 Info:  * Sub-Job
 Info:  * Sub-Job
diff --git a/test/build-cli-29.testcase b/test/build-cli-29.testcase
index 747a56f..c927bae 100644
--- a/test/build-cli-29.testcase
+++ b/test/build-cli-29.testcase
@@ -25,9 +25,9 @@ Info: Generatiing pkginfo.manifest...
 Info: Zipping...
 Info: Creating package file ... a_0.0.5_ubuntu-32.zip
 Info: Checking reverse build dependency ...
-Info:  * Will check reverse-build for projects: testb(ubuntu-32), testc(ubuntu-32)
-Info:  * Added new job for reverse-build ... 				testb(ubuntu-32)
-Info:  * Added new job for reverse-build ... 				testc(ubuntu-32)
+Info:  * Will check reverse-build for projects: 
+Info:  * Added new job for reverse-build ...
+Info:  * Added new job for reverse-build ...
 Info:  * Reverse-build OK ...
 Info:  * Reverse-build OK ...
 Info: Uploading ...