makepkginfo shouldn't fail if dmg is already mounted

[GoogleCodeExporter]

Currently when running makepkginfo on a disk image that is already mounted, the 
operation fails with errors from hdiutil. Should this be changed so that if the 
image is already mounted, makepkginfo could run 'hdiutil info' to get the mount 
point and use that?

I suspect there will be edge cases with this but I think the change in 
behaviour would be what the user expects makepkginfo to do.

I've committed some changes in my GitHub clone to address this:
https://github.com/hjuutilainen/munki/commit/8f458abd28d74216840e046a25d31bf8a50
e8455

--
Hannes Juutilainen

Original issue reported on code.google.com by hjuutila...@mac.com on 13 Mar 2013 at 9:11

[GoogleCodeExporter]

And now i started to think this further. What if makepkginfo could also take 
disk image mount points as input and resolve them to a dmg file?

I mean running it like this (manually mounted Xcode auxiliary tools dmg before):
makepkginfo "/Volumes/Auxiliary Tools"
makepkginfo "/Volumes/Auxiliary Tools" -i "/Volumes/Auxiliary 
Tools/CrashReporterPrefs.app"

Testing for this functionality in commit:
https://github.com/hjuutilainen/munki/commit/24d43c22da06952d4911597261f005f8302
d87d9

--
Hannes

Original comment by hjuutila...@mac.com on 13 Mar 2013 at 12:08

[GoogleCodeExporter]

Here's my concern:

Your changes modify the behavior of munkicommon.mountdmg(), which is used by 
Munki when installing (and in some cases removing) items. Are we sure these 
changes won't affect installs? I'm not sure we want to proceed if a diskimage 
Munki has downloaded and intends to use for installation is unexpectedly 
already mounted; and even if we do, what's the expected behavior later when it 
comes time to unmount the disk image and remove it?

Perhaps there needs to be another flag munkicommon.mountdmg (that defaults to 
False) that tells it whether or not to return an existing mount point. This 
way, current behavior is maintained for the main Munki code, and we are not 
introducing a possibly risky change into that code simply to help a sloppy 
admin using makepkginfo.

-Greg

Original comment by gregnea...@mac.com on 13 Mar 2013 at 2:03

[GoogleCodeExporter]

Do these changes work properly with munkiimport?

-Greg

Original comment by gregnea...@mac.com on 13 Mar 2013 at 2:05

[GoogleCodeExporter]

I absolutely share your concerns and the munkicommon.mountdmg() is probably not 
be the best place to do this. As you said, this change would only benefit 
makepkginfo and modifying current behavior for everything else is not wanted. 
I'll find a better way to implement this. But in general, you're not opposed to 
this?

"...sloppy admin using makepkginfo"
I don't think this has anything to do with sloppy admins. There are numerous 
situations where you need to see the contents of the dmg to be able to provide 
the correct options for makepkginfo. Requiring a detach before running the 
command is pointless.

"Do these changes work properly with munkiimport?"
Don't know. Haven't tested yet as I'm not using it with my main repos. It 
definitely needs to.

--
Hannes

Original comment by hjuutila...@mac.com on 13 Mar 2013 at 4:05

[GoogleCodeExporter]

"But in general, you're not opposed to this?" 

No.

""...sloppy admin using makepkginfo"
I don't think this has anything to do with sloppy admins. "

I wasn't sure what your use-case was, but I think I understand it now. Yes, it 
has been a pain to use munkiimport or makepkginfo to generate a pkginfo with -i 
or -p items; it was almost always a multi-step, manual process. This set of 
changes should make it much easier.

And yes, it's important that they changes work with munkiimport, as I'm pretty 
sure most Munki admins that aren't using MunkiAdmin or MunkiServer, but are 
working from the command-line are using munkiimport primarily and dropping down 
to makepkginfo only when needed.

Original comment by gregnea...@mac.com on 13 Mar 2013 at 4:16

[GoogleCodeExporter]

Yes, this all started as a convenience for -i items and the main purpose would 
be to make the workflow a bit easier. 

Give me a day or two and I'll come up with a tested and safer way to do this. 
And I'll make sure munkiimport works too :)

Original comment by hjuutila...@mac.com on 13 Mar 2013 at 4:42

[GoogleCodeExporter]

Ok, after some scratching I think the easiest way to do this would be to just 
add an optional use_existing_mounts argument to mountdmg() which defaults to 
false. Then just call this from makepkginfo with munkicommon.mountdmg(dmgpath, 
use_existing_mounts=True)

This change is in:
https://github.com/hjuutilainen/munki/commit/f3da0ce63a735d13912d878f4902b779a68
2bc78

I also did some brief testing with munkiimport and it seems to be working. I'll 
need to test all of this some more but what do you think about the current 
changes?

Latest changes for all of these can be found in the development branch head:
https://github.com/hjuutilainen/munki/commit/8ca86a0d1c54121974d5be94ec8683fa07a
097be

--
Hannes

Original comment by hjuutila...@mac.com on 14 Mar 2013 at 1:38

[GoogleCodeExporter]

Are you happy with the current state of these changes?

Original comment by gregnea...@mac.com on 15 Apr 2013 at 11:33

[GoogleCodeExporter]

Yes, I'm happy. Do you think the current implementation is safe enough?

Original comment by hjuutila...@mac.com on 16 Apr 2013 at 11:18

[GoogleCodeExporter]

This was merged a couple of months ago: 
https://code.google.com/p/munki/source/detail?r=2ce6f6127cfac4d4c87062019c7d85c7
a71433a4 so updating status

Original comment by gregnea...@mac.com on 5 Nov 2013 at 11:08

• Changed state: Fixed