Notes |
|
(0028964)
|
Eric NOULARD
|
2012-03-25 17:46
|
|
I think we cannot do that like that because it would certainly
make CPack test go red for every MacOS X dashboard.
i.e. CPackTestAllGenerators is run on every enabled generators
(loop is done over the list provided by "cpack --help")
As far as I understand Homebrew is not systematically installed on a Mac.
So we should probably only "enable" CPackRPM if requirement are installed
i.e. if rpmbuild is found.
Currently for the DEB generator it's different because it can work
without dpkg tools.
Alternatively we could enable RPM and DEB but throw a proper warning/error
if they cannot be run because some tools are missing. |
|
|
(0028967)
|
Tom Hughes
|
2012-03-26 02:37
|
|
You are correct that Homebrew is not systematically installed on a Mac.
>> Alternatively we could enable RPM and DEB but throw a proper warning/error
>> if they cannot be run because some tools are missing.
I think that makes sense. |
|
|
(0028981)
|
Tom Hughes
|
2012-03-27 12:28
|
|
Thinking about this a bit more, "rpmbuild" is not installed on many Linux distributions by default, so wouldn't the same issue apply to it? (e.g., Ubuntu does not provide rpmbuild by default and it is not a dependency of the cmake package) |
|
|
(0028985)
|
Eric NOULARD
|
2012-03-27 16:06
|
|
Yes you are right.
After more thorough check CPackTestAllGenerators does not check
on return code so that if the CPack generator fails it is not
an error...
Nevertheless, having done a mistake in the past does not mean
we want to continue along that path in the future :-]
CPackRPM checks for rpm/rpmbuild command when launched and sends
an error message if it is not found. This is OK but
not satisfactory in my opinion. I'd rather do that as early as possible
like not adding the generator to the factory if the tool is not available.
So in the end
I'd rather prefer to jump on your request in order to do something "better"
than what we do until now.
something like:
cmSystemTools::FindProgram(rpmbuild) should be enough for CPackRPM.
May be it would be even better to add some ?static? method to
cmCPack Generator sub-classes for checking if adding the sub-class to the
factory is OK or not.
I'll have a look at the code, unless you want to do that yourself?
(I'm ok since I cannot ensure any deadline for that). |
|
|
(0028994)
|
Tom Hughes
|
2012-03-29 01:28
|
|
How about something like the most recent patch I attached? (with the original patch as well) |
|
|
(0029003)
|
Eric NOULARD
|
2012-03-30 09:07
|
|
Looks good to me.
I'll test a little bit. |
|
|
(0029006)
|
Eric NOULARD
|
2012-03-30 11:17
|
|
I did add CPack tests update and put the proposal on stage:
To git@cmake.org:stage/cmake.git
* [new branch] HEAD -> CPack-activateRPM-DEB-onMacOS
I'd like others to review the work before pushing this to next.
We may have kind of non-backward compatible behavior that will make
cpack fail in a different manner than before.
e.g. previously if you were on Linux without "rpmbuild" installed
you'll get an error give by CPackRPM generator telling you
need to have rpmbuild.
Now you'll get a CPack generic generator error telling you that the
generator does not exist.
I'm thinking of a scheme which would make it possible to have a fully backward compatible behavior or at least indicate that "CPackRPM" cannot be run
because "rpmbuild is not there". |
|
|
(0029007)
|
Eric NOULARD
|
2012-03-30 11:18
|
|
I attached a patch corresponding to the cumulative modifications (yours and mine). |
|
|
(0029141)
|
Tom Hughes
|
2012-04-13 01:33
|
|
Any updates on the patch review? |
|
|
(0029166)
|
Eric NOULARD
|
2012-04-16 06:42
|
|
No news.
I was on vacation (mostly off network) until now.
The stage/CPack-activateRPM-DEB-onMacOS is still there
but I didn't have any feedback before my vacation
so I didn't merged it.
May be you could ping on cmake-developer mailing list but
since 2.8.8 rc series is almost over I think we should wait for
2.8.9 work before merging this in.
Merging other things than "regression fix" when entering rc's is
usually not a good idea. |
|
|
(0029471)
|
Tom Hughes
|
2012-05-14 16:36
|
|
I tried sending an email to the developer mailing list last month, but never got a response.
Eric, do you think you could merge it in and see if anyone objects? |
|
|
(0029472)
|
Eric NOULARD
|
2012-05-14 16:41
|
|
I'll try a merge on Thursday.
That way I have 1 or 2 days to fix or revert if half the dashboard goes red. |
|
|
(0029514)
|
Eric NOULARD
|
2012-05-20 16:10
|
|
Hi Tom,
I rework the branch a little in order to ensure fully backward compatible
behavior on platform where RPM and DEB were (statically) enabled before.
I deleted the previous branch and re-create a clean history on a new branch
with the same name.
Just merged to next:
Merge topic 'CPack-activateRPM-DEB-onMacOS' into next
2a34b57 CPack allow RPM and DEB generator to be used on OSX.
We will see what Kitware guy's think about that in the next merging session. |
|
|
(0030661)
|
Eric NOULARD
|
2012-08-14 17:33
|
|
This has been included in the 2.8.9 release. |
|