cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Erik Weber" <terbol...@gmail.com>
Subject Review Request 24204: CLOUDSTACK-7230 Pass 'noredist' as lower case in package.sh and updated cloud.spec
Date Sat, 02 Aug 2014 13:49:50 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24204/
-----------------------------------------------------------

Review request for cloudstack and Hugo Trippaers.


Bugs: CLOUDSTACK-7230
    https://issues.apache.org/jira/browse/CLOUDSTACK-7230


Repository: cloudstack-git


Description
-------

Steps to reproduce:

1) checkout master
2) cd packaging/centos63; ./package.sh -p noredist
3) verify that cloudstack-mysql-ha rpm is not created


Currently package/centos63/package.sh claims that you can pass '-p noredist|NOREDIST' to build
the noredist packages.

The value you pass it, will be reused later in the function packaging due to this line:
        DEFOSSNOSS="-D_ossnoss $packageval"

$packageval is set to the input parameter earlier before being passed to the packaging function.


This means, that if you execute package.sh with the parameter '-p noredist', then rpmbuild
will receive it as lower case, but if you execute it with parameter '-p NOREDIST' rpmbuild
will receive it as 'NOREDIST'.

There are a couple of if statements in the cloud.spec that checks for 'NOREDIST' or 'noredist',
but the introduction of the mysql-ha package only checks for 'NOREDIST', meaning that if you
pass it as 'noredist' you will not get the mysql-ha package built.

I changed package.sh to send the parameter to rpmbuild as lower case, but package.sh still
allows to be passed both.
I also changed the upper case only if's in cloud.spec for mysql-ha, to lower case so that
they suit the new package.sh script.

There is a tradeoff here, anyone that does packaging any other way than using package.sh might
have to change to passing 'NOREDIST' as lower case.
For the future we might consider if the cloud.spec should handle lower casing the parameter,
if possible. I haven't checked into that possibility yet.

If you want me to rather change the cloud.spec to check for both upper and lower case, let
me know and I'll update the patch.


Diffs
-----

  packaging/centos63/cloud.spec 1d4d061 
  packaging/centos63/package.sh 07f95fc 

Diff: https://reviews.apache.org/r/24204/diff/


Testing
-------

Verified that noredist packages, including mysql-ha, gets built when passing either 'NOREDIST'
or 'noredist' as parameter to package.sh


File Attachments
----------------

CLOUDSTACK-7230
  https://reviews.apache.org/media/uploaded/files/2014/08/02/6d5e4454-3f75-454c-8688-e3314acbc043__CLOUDSTACK-7230.patch


Thanks,

Erik Weber


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message