Skip to content

Patch installation fixes #22

Merged
merged 18 commits into from Jan 3, 2013

3 participants

@lslezak
webyast member
lslezak commented Dec 19, 2012

Fixes related to installing patches with EULA (license):

  • Properly track the patch <-> EULA mapping (save also patch and package ID when saving license text), display package name when displaying EULA, remove only the relevant EULA after patch installation (not all as before).
    This avoids problems when installing more patches with EULA as we now know which EULA belongs to which patch.

  • Directly install the patch when accepting the EULA ("Accept" -> "Accept and Install" button), no need to go back to the patch selection and install it once again. This also avoids displaying an error after accepting the EULA (patch installation failed because of missing EULA agreement) so the user is not confused.

  • Use File.move or File.delete for copying and moving instead of shell commands - faster, no fork and shell exec needed, secure (no problem with escaping file names)

  • Added logging

@vmoravec vmoravec was assigned Dec 19, 2012
@vmoravec vmoravec commented on an outdated diff Dec 20, 2012
plugins/software/app/models/patch.rb
Rails.logger.info "EULA #{eula_id.inspect} is required for #{package_id.inspect}"
- #FIXME check if user already agree with license
- create_eula(eula_id,license_text)
+ create_eula(eula_id, package_id, pk_id, license_text)
@vmoravec
vmoravec added a note Dec 20, 2012

Is the name of the variable pk_id correct? The method create_eula calls this variable patch_id .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@vmoravec vmoravec commented on an outdated diff Dec 20, 2012
plugins/software/app/models/patch.rb
dir = Dir.new(ACCEPTED_LICENSES_DIR)
- dir.each {|filename|
- File.delete(File.join(ACCEPTED_LICENSES_DIR,filename)) unless File.directory? filename
- }
+ dir.each do |filename|
+ unless File.directory? filename
+ license_file = File.join(ACCEPTED_LICENSES_DIR, filename)
+ package_id, patch_id, text = File.read(license_file).split("\n", 3)
@vmoravec
vmoravec added a note Dec 20, 2012

Why 3? It's used in the license method too. Perhaps a separate method with a clear name like read_license along with a comment why there is this integer needed would make it more transparent and would make it DRY as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
lslezak added some commits Dec 20, 2012
@lslezak lslezak stop patch installation on EULA request install patches after accepti…
…ng the EULA

packagekit is locked and does not install other patches until the license is accepted
4b567d3
@lslezak lslezak do not query packagekit again if we already have the patch ID
added logging
3d6c18f
@lslezak lslezak workaround: read available patches from cache
packagekit sometimes(?) does not report patch which EULA is not confirmed
2e526c5
@vlewin vlewin was assigned Jan 2, 2013
@vmoravec vmoravec merged commit 2c8108f into master Jan 3, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.