Enable plugins to munki that will allow writes to the munki repo to be customized. #689

Closed
wants to merge 20 commits into
from

Conversation

Projects
None yet
2 participants
@ryanyu91
Contributor

ryanyu91 commented Jan 6, 2017

NEW CHANGES

Commit based on comments by Greg Neagle on what to fix: munki#685 (master...ryanyu91:master)
IconImporter:

Not opening DMG and then mounting it anymore, directly mounting like how it was before
ManifestUtil:
Checking if the repo is mounted as well as if we (munki) mounted it. Only this will display prompt whether we want to unmount or not
FileRepo.py, Repo.py:
not hardcoding import path anymore
FileRepo - added 10.12 mounting fileshares code
Changed Files:
code/client/munkiimport
code/client/iconimporter
code/client/makecatalogs
code/client/manifestutil
code/client/munkilib/iconutils.py

Added Files:
code/client/munkilib/FileRepo.py
code/client/munkilib/Repo.py

Reason For Changes:

The purpose for this change is to enable plugins to munki that will allow writes to the munki repo to be customized.

Changes:

The methods used to write to the munki repo are the target of this modification. A plugin can create new methods for overwriting the default behavior of writing to the munki repo.

The default behavior is retained in the absence of a plugin. This is accomplished via the introduction of FileRepo.py which continues to simply write changes to the munki repo. A plugin can be introduced to change this default behavior and allow munki repo writes to be redirected as desired by the plugin author.

In order to accomplish this we have refactored all of the os.path.* methods, as well as the mount, unmount and available methods from the following tools: (iconimporter, makecatalogs, munkiimport, manifestutil, iconutils), and put them into a different python script to be used as a library or common code module. By default, these are now in the FileRepo.py module. The plugin can be used to overwrite this common code module.

In addition to the above described changes, we also made the following changes to support this plugin
concept:
• Added the ability to add a plugin either via the command line or via munkiimport –configure
• The code will look for custom plugins in the /usr/local/munki/munkilib/plugins/ directory
• If plugin is found in the plugins directory, munkiimport --configure will give the option to type in the plugin name. For example, if you plugin is Foo.py, you could specify a plugin name of Foo in munkiimport --configure
• If no plugin is found the FileRepo.py module will be used as the default common code module for writing to the munki repo
• Add the ability to set the plugin via the --plugin option for the following tools (makecatalogs, munkiimport, manifestutil)

Testing
Tested with munkiimport on local filesystem/network shares/our own custom plugins
Tested manifestutil on local filesystem/network
Verified through regression testing, all features of munkiimport, makecatalogs, iconimporter, manifestutil from before all still work (local filesystem/network shares)

ryanyu91 added some commits Dec 12, 2016

-Refactor Repo methods into FileRepo and Repo
-added plugin option and configure for custom imports
-Updated munkiimport to latest version
Refactored iconimporter, makecatalogs, manifestutil, iconutils Repo m…
…ethods to Repo/FileRepo to support custom plugin functionality
Pull request comments by Greg Neagle on what to fix: #685
IconImporter:
	- Not opening DMG and then mounting it anymore, directly mounting like how it was before
ManifestUtil:
	- Checking if the repo is mounted as well as if we (munki) mounted it. Only this will display prompt whether we want to unmount or not
FileRepo.py, Repo.py:
	- not hardcoding import path anymore
	- FileRepo - added 10.12 mounting fileshares code
@gregneagle

This comment has been minimized.

Show comment
Hide comment
@gregneagle

gregneagle Jan 12, 2017

Contributor

Doing some testing. Ran makecatalogs from this PR, got this odd output:

<snip>
Adding utilities/Oracle/OracleJava7JDK-1.7.55.13.plist to production...
Adding utility/Oracle/OracleJava8JDK-1.8.0_101.plist to production...
Adding utility/Oracle/OracleJava8JDK-1.8.0_111.plist to production...
Adding utility/Oracle/OracleJava8JDK-1.8.0_111.plist~ to production...
Adding utility/Oracle/OracleJava8JDK-1.8.0_91.plist to production...
<munkilib.Repo.Repo instance at 0x10e73e560>
catalogs

Created catalog catalogs/development...
Created catalog catalogs/all...
Created catalog catalogs/swstage...
<snip>

is the

<munkilib.Repo.Repo instance at 0x10e73e560>
catalogs

bit some left-over debug output? I'll try to look for the cause, but I hope you'll look, too.

Contributor

gregneagle commented Jan 12, 2017

Doing some testing. Ran makecatalogs from this PR, got this odd output:

<snip>
Adding utilities/Oracle/OracleJava7JDK-1.7.55.13.plist to production...
Adding utility/Oracle/OracleJava8JDK-1.8.0_101.plist to production...
Adding utility/Oracle/OracleJava8JDK-1.8.0_111.plist to production...
Adding utility/Oracle/OracleJava8JDK-1.8.0_111.plist~ to production...
Adding utility/Oracle/OracleJava8JDK-1.8.0_91.plist to production...
<munkilib.Repo.Repo instance at 0x10e73e560>
catalogs

Created catalog catalogs/development...
Created catalog catalogs/all...
Created catalog catalogs/swstage...
<snip>

is the

<munkilib.Repo.Repo instance at 0x10e73e560>
catalogs

bit some left-over debug output? I'll try to look for the cause, but I hope you'll look, too.

@gregneagle

This comment has been minimized.

Show comment
Hide comment
@gregneagle

gregneagle Jan 12, 2017

Contributor

After looking:

https://github.com/ryanyu91/munki/blob/master/code/client/makecatalogs#L332-L333

It does appear to be leftover debug code.

Contributor

gregneagle commented Jan 12, 2017

After looking:

https://github.com/ryanyu91/munki/blob/master/code/client/makecatalogs#L332-L333

It does appear to be leftover debug code.

@gregneagle

This comment has been minimized.

Show comment
Hide comment
@gregneagle

gregneagle Jan 12, 2017

Contributor

Next issue:

% ./manifestutil --help
Unknown subcommand: --help
Available sub-commands:
	add-catalog
	add-included-manifest
	add-pkg
	configure
	copy-manifest
	display-manifest
	exit
	expand-included-manifests
	find
	help
	list-catalog-items
	list-catalogs
	list-manifests
	new-manifest
	refresh-cache
	remove-catalog
	remove-included-manifest
	remove-pkg
	rename-manifest
	version
Traceback (most recent call last):
  File "./manifestutil", line 1238, in <module>
    main()
  File "./manifestutil", line 1206, in main
    cleanup_and_exit(retcode)
  File "./manifestutil", line 427, in cleanup_and_exit
    if repo.mounted and WE_MOUNTED_THE_REPO:
AttributeError: 'NoneType' object has no attribute 'mounted'

repo hasn't been initialized yet, so the line probably should read

if repo and repo.mounted and WE_MOUNTED_THE_REPO:

Contributor

gregneagle commented Jan 12, 2017

Next issue:

% ./manifestutil --help
Unknown subcommand: --help
Available sub-commands:
	add-catalog
	add-included-manifest
	add-pkg
	configure
	copy-manifest
	display-manifest
	exit
	expand-included-manifests
	find
	help
	list-catalog-items
	list-catalogs
	list-manifests
	new-manifest
	refresh-cache
	remove-catalog
	remove-included-manifest
	remove-pkg
	rename-manifest
	version
Traceback (most recent call last):
  File "./manifestutil", line 1238, in <module>
    main()
  File "./manifestutil", line 1206, in main
    cleanup_and_exit(retcode)
  File "./manifestutil", line 427, in cleanup_and_exit
    if repo.mounted and WE_MOUNTED_THE_REPO:
AttributeError: 'NoneType' object has no attribute 'mounted'

repo hasn't been initialized yet, so the line probably should read

if repo and repo.mounted and WE_MOUNTED_THE_REPO:

@gregneagle

This comment has been minimized.

Show comment
Hide comment
@gregneagle

gregneagle Jan 12, 2017

Contributor

Next issue/error:

No existing product icon found.
Attempt to create a product icon? [y/n] n
Copying DA_sshConfig-1.7.pkg to pkgs/disney/DA_sshConfig-1.7.pkg...
Traceback (most recent call last):
  File "./munkiimport", line 1238, in <module>
    main()
  File "./munkiimport", line 1214, in main
    add_icon_hash_to_pkginfo(pkginfo)
  File "./munkiimport", line 316, in add_icon_hash_to_pkginfo
    handle = repo.open(icon_path, 'r')
  File "/Users/Shared/ryanyu91/munki/code/client/munkilib/FileRepo.py", line 221, in open
    return RepoFile(self, os.path.join(self.path, path), mode)
  File "/Users/Shared/ryanyu91/munki/code/client/munkilib/FileRepo.py", line 215, in __init__
    self.file = open(self.repo_path, mode)
IOError: [Errno 2] No such file or directory: u'/Volumes/repo/icons/DA_sshConfig.png'

compare the behavior for the release code:

No existing product icon found.
Attempt to create a product icon? [y/n] n
Copying DA_sshConfig-1.7.pkg to /Volumes/repo/pkgs/disney/DA_sshConfig-1.7.pkg...
Saving pkginfo to /Volumes/repo/pkgsinfo/disney/DA_sshConfig-1.7.plist...
Rebuild catalogs? [y/n]
Contributor

gregneagle commented Jan 12, 2017

Next issue/error:

No existing product icon found.
Attempt to create a product icon? [y/n] n
Copying DA_sshConfig-1.7.pkg to pkgs/disney/DA_sshConfig-1.7.pkg...
Traceback (most recent call last):
  File "./munkiimport", line 1238, in <module>
    main()
  File "./munkiimport", line 1214, in main
    add_icon_hash_to_pkginfo(pkginfo)
  File "./munkiimport", line 316, in add_icon_hash_to_pkginfo
    handle = repo.open(icon_path, 'r')
  File "/Users/Shared/ryanyu91/munki/code/client/munkilib/FileRepo.py", line 221, in open
    return RepoFile(self, os.path.join(self.path, path), mode)
  File "/Users/Shared/ryanyu91/munki/code/client/munkilib/FileRepo.py", line 215, in __init__
    self.file = open(self.repo_path, mode)
IOError: [Errno 2] No such file or directory: u'/Volumes/repo/icons/DA_sshConfig.png'

compare the behavior for the release code:

No existing product icon found.
Attempt to create a product icon? [y/n] n
Copying DA_sshConfig-1.7.pkg to /Volumes/repo/pkgs/disney/DA_sshConfig-1.7.pkg...
Saving pkginfo to /Volumes/repo/pkgsinfo/disney/DA_sshConfig-1.7.plist...
Rebuild catalogs? [y/n]
@gregneagle

This comment has been minimized.

Show comment
Hide comment
@gregneagle

gregneagle Jan 12, 2017

Contributor

For this last one, it seems that this is caused because the python os.path.isfile(pathname) and os.path.isdir(pathname) etc methods return False if pathname doesn't exist. Your code attempts to open the file first:

https://github.com/ryanyu91/munki/blob/master/code/client/munkiimport#L316

if the file doesn't exist, instead of a None handle, we get an IOError exception.

Contributor

gregneagle commented Jan 12, 2017

For this last one, it seems that this is caused because the python os.path.isfile(pathname) and os.path.isdir(pathname) etc methods return False if pathname doesn't exist. Your code attempts to open the file first:

https://github.com/ryanyu91/munki/blob/master/code/client/munkiimport#L316

if the file doesn't exist, instead of a None handle, we get an IOError exception.

code/client/munkilib/FileRepo.py
+try:
+ import errno
+ import getpass
+ import objc

This comment has been minimized.

@gregneagle

gregneagle Jan 13, 2017

Contributor

Second import of objc

@gregneagle

gregneagle Jan 13, 2017

Contributor

Second import of objc

code/client/munkilib/FileRepo.py
+ password = getpass.getpass()
+ mount_share_with_credentials(share_url, username, password)
+
+class FileRepo:

This comment has been minimized.

@gregneagle

gregneagle Jan 13, 2017

Contributor

Any reason this isn't class FileRepo(object): -- as is this defines an "Old-style class". Is that what you intended? If so, why?

@gregneagle

gregneagle Jan 13, 2017

Contributor

Any reason this isn't class FileRepo(object): -- as is this defines an "Old-style class". Is that what you intended? If so, why?

code/client/munkilib/FileRepo.py
+ original_dir = os.getcwd()
+ os.chdir(path)
+ for arg in args:
+ pkgs += glob.glob(arg)

This comment has been minimized.

@gregneagle

gregneagle Jan 13, 2017

Contributor

pkgs is not yet defined (maybe this should be matches?), and I think we're missing import glob in the imports section. Should this also return something (again, perhaps matches...)?

@gregneagle

gregneagle Jan 13, 2017

Contributor

pkgs is not yet defined (maybe this should be matches?), and I think we're missing import glob in the imports section. Should this also return something (again, perhaps matches...)?

code/client/munkilib/FileRepo.py
+
+ def mount(self):
+ '''Mounts the repo locally.'''
+ global WE_MOUNTED_THE_REPO

This comment has been minimized.

@gregneagle

gregneagle Jan 13, 2017

Contributor

If you mark this as a global, you need to define it at the module level. But since you have a class now, this should probably be an object attribute.

@gregneagle

gregneagle Jan 13, 2017

Contributor

If you mark this as a global, you need to define it at the module level. But since you have a class now, this should probably be an object attribute.

code/client/munkilib/FileRepo.py
+a remote repo mounted via AFP, SMB, or NFS.
+"""
+
+from collections import namedtuple

This comment has been minimized.

@gregneagle

gregneagle Jan 13, 2017

Contributor

This import is never used.

@gregneagle

gregneagle Jan 13, 2017

Contributor

This import is never used.

code/client/iconimporter
+ print_utf8("Using repo url: %s" % repo_url)
+
+ # Make sure the repo exists
+ repo = Repo.Open(repo_path, repo_url)

This comment has been minimized.

@gregneagle

gregneagle Jan 13, 2017

Contributor

Repo.Open() is defined with three parameters: path, url, and plugin. If you intend the plugin to be optional, it should be defined differently.

@gregneagle

gregneagle Jan 13, 2017

Contributor

Repo.Open() is defined with three parameters: path, url, and plugin. If you intend the plugin to be optional, it should be defined differently.

if result:
print_utf8(u'\tWrote: %s' % png_path)
index += 1
else:
print_utf8(u'\tNo application icons found.')
-def find_items_to_check(repo_path, itemlist=None):
+def findItemsToCheck(repo, itemlist=None):

This comment has been minimized.

@gregneagle

gregneagle Jan 13, 2017

Contributor

This function used to be named "find_items_to_check". The new name causes a problem later in the code...

@gregneagle

gregneagle Jan 13, 2017

Contributor

This function used to be named "find_items_to_check". The new name causes a problem later in the code...

@gregneagle

See individual line comments and also PR-level comments.

code/client/iconimporter
- icons_dir = os.path.join(repo_path, u'icons')
- if not os.path.exists(icons_dir):
- os.mkdir(icons_dir)
+ itemlist = find_items_to_check(repo, itemlist=itemlist)

This comment has been minimized.

@gregneagle

gregneagle Jan 13, 2017

Contributor

the find_items_to_check function no longer exists since it was renamed to findItemsToCheck

@gregneagle

gregneagle Jan 13, 2017

Contributor

the find_items_to_check function no longer exists since it was renamed to findItemsToCheck

ryanyu91 added some commits Jan 16, 2017

NEW CHANGES
************************************************************************************************************************************************************************************
#689 (review)
Commit based on comments by Greg Neagle on what to fix: munki#689 (master...ryanyu91:master)

makecatalogs:
removed debugging print statements

manifestutil:
check if repo is initialized before checking if it is mounted

munkiimport:
check if repo is initialized before checking if it is mounted
for def add_icon_hash_to_pkginfo(pkginfo):, not opening file (icon_path) anymore
WE_MOUNTED_THE_REPO is a repo attribute now, instead of a global variable

FileRepo:
removed duplicate/unused imports
changed class to not be old-style
updated glob method since pkgs was uninitialized
made WE_MOUNTED_THE_REPO from global variable to FileRepo attribute variable initialized to False

iconimporter:
updated Repo call to include 3 parameters now..
changed old find_items_to_check calls to new call: findItemsToCheck

Repo.py
Added support for relative paths for importing plugins
************************************************************************************************************************************************************************************

Commit based on comments by Greg Neagle on what to fix: munki#685 (master...ryanyu91:master)
IconImporter:

Not opening DMG and then mounting it anymore, directly mounting like how it was before
ManifestUtil:
Checking if the repo is mounted as well as if we (munki) mounted it. Only this will display prompt whether we want to unmount or not
FileRepo.py, Repo.py:
not hardcoding import path anymore
FileRepo - added 10.12 mounting fileshares code
************************************************************************************************************************************************************************************

Changed Files:
code/client/munkiimport
code/client/iconimporter
code/client/makecatalogs
code/client/manifestutil
code/client/munkilib/iconutils.py

Added Files:
code/client/munkilib/FileRepo.py
code/client/munkilib/Repo.py

Reason For Changes:

The purpose for this change is to enable plugins to munki that will allow writes to the munki repo to be customized.

Changes:

The methods used to write to the munki repo are the target of this modification. A plugin can create new methods for overwriting the default behavior of writing to the munki repo.

The default behavior is retained in the absence of a plugin. This is accomplished via the introduction of FileRepo.py which continues to simply write changes to the munki repo. A plugin can be introduced to change this default behavior and allow munki repo writes to be redirected as desired by the plugin author.

In order to accomplish this we have refactored all of the os.path.* methods, as well as the mount, unmount and available methods from the following tools: (iconimporter, makecatalogs, munkiimport, manifestutil, iconutils), and put them into a different python script to be used as a library or common code module. By default, these are now in the FileRepo.py module. The plugin can be used to overwrite this common code module.

In addition to the above described changes, we also made the following changes to support this plugin
concept:
•	Added the ability to add a plugin either via the command line or via munkiimport –configure
•	The code will look for custom plugins in the /usr/local/munki/munkilib/plugins/ directory
•	If plugin is found in the plugins directory, munkiimport --configure will give the option to type in the plugin name. For example, if you plugin is Foo.py, you could specify a plugin name of Foo in munkiimport --configure
•	If no plugin is found the FileRepo.py module will be used as the default common code module for writing to the munki repo
•	Add the ability to set the plugin via the --plugin option for the following tools (makecatalogs, munkiimport, manifestutil)

Testing
Tested with munkiimport on local filesystem/network shares/our own custom plugins
Tested manifestutil on local filesystem/network
Verified through regression testing, all features of munkiimport, makecatalogs, iconimporter, manifestutil from before all still work (local filesystem/network shares)
Revert "NEW CHANGES"
This reverts commit 543613a.
NEW CHANGES
************************************************************************************************************************************************************************************
#689 (review)
Commit based on comments by Greg Neagle on what to fix: munki#689 (master...ryanyu91:master)

makecatalogs:
removed debugging print statements

manifestutil:
check if repo is initialized before checking if it is mounted

munkiimport:
check if repo is initialized before checking if it is mounted
for def add_icon_hash_to_pkginfo(pkginfo):, not opening file (icon_path) anymore
WE_MOUNTED_THE_REPO is a repo attribute now, instead of a global variable

FileRepo:
removed duplicate/unused imports
changed class to not be old-style
updated glob method since pkgs was uninitialized
made WE_MOUNTED_THE_REPO from global variable to FileRepo attribute variable initialized to False

iconimporter:
updated Repo call to include 3 parameters now..
changed old find_items_to_check calls to new call: findItemsToCheck

Repo.py
Added support for relative paths for importing plugins
************************************************************************************************************************************************************************************

Commit based on comments by Greg Neagle on what to fix: munki#685 (master...ryanyu91:master)
IconImporter:

Not opening DMG and then mounting it anymore, directly mounting like how it was before
ManifestUtil:
Checking if the repo is mounted as well as if we (munki) mounted it. Only this will display prompt whether we want to unmount or not
FileRepo.py, Repo.py:
not hardcoding import path anymore
FileRepo - added 10.12 mounting fileshares code
************************************************************************************************************************************************************************************

Changed Files:
code/client/munkiimport
code/client/iconimporter
code/client/makecatalogs
code/client/manifestutil
code/client/munkilib/iconutils.py

Added Files:
code/client/munkilib/FileRepo.py
code/client/munkilib/Repo.py

Reason For Changes:

The purpose for this change is to enable plugins to munki that will allow writes to the munki repo to be customized.

Changes:

The methods used to write to the munki repo are the target of this modification. A plugin can create new methods for overwriting the default behavior of writing to the munki repo.

The default behavior is retained in the absence of a plugin. This is accomplished via the introduction of FileRepo.py which continues to simply write changes to the munki repo. A plugin can be introduced to change this default behavior and allow munki repo writes to be redirected as desired by the plugin author.

In order to accomplish this we have refactored all of the os.path.* methods, as well as the mount, unmount and available methods from the following tools: (iconimporter, makecatalogs, munkiimport, manifestutil, iconutils), and put them into a different python script to be used as a library or common code module. By default, these are now in the FileRepo.py module. The plugin can be used to overwrite this common code module.

In addition to the above described changes, we also made the following changes to support this plugin
concept:
•	Added the ability to add a plugin either via the command line or via munkiimport –configure
•	The code will look for custom plugins in the /usr/local/munki/munkilib/plugins/ directory
•	If plugin is found in the plugins directory, munkiimport --configure will give the option to type in the plugin name. For example, if you plugin is Foo.py, you could specify a plugin name of Foo in munkiimport --configure
•	If no plugin is found the FileRepo.py module will be used as the default common code module for writing to the munki repo
•	Add the ability to set the plugin via the --plugin option for the following tools (makecatalogs, munkiimport, manifestutil)

Testing
Tested with munkiimport on local filesystem/network shares/our own custom plugins
Tested manifestutil on local filesystem/network
Verified through regression testing, all features of munkiimport, makecatalogs, iconimporter, manifestutil from before all still work (local filesystem/network shares)
Merge branch 'master' of https://github.com/ryanyu91/munki
# Conflicts:
#	code/client/munkilib/Repo.py
@ryanyu91

This comment has been minimized.

Show comment
Hide comment
@ryanyu91

ryanyu91 Jan 16, 2017

Contributor

Hi Greg, Thanks for your review. I updated the files based on your comments, they're in 543613a

Contributor

ryanyu91 commented Jan 16, 2017

Hi Greg, Thanks for your review. I updated the files based on your comments, they're in 543613a

@gregneagle

This comment has been minimized.

Show comment
Hide comment
@gregneagle

gregneagle Jan 18, 2017

Contributor

Pulled latest changes.

% ./makecatalogs 
Using repo path: /Volumes/repo
Traceback (most recent call last):
  File "./makecatalogs", line 429, in <module>
    main()
  File "./makecatalogs", line 420, in main
    repo = Repo.Open(repopath, options.repo_url, options.plugin)
  File "/Users/Shared/ryanyu91/munki/code/client/munkilib/Repo.py", line 39, in Open
    module = imp.load_source('FileRepo', commandPath+'/munkilib/FileRepo.py')
IOError: [Errno 2] No such file or directory
Contributor

gregneagle commented Jan 18, 2017

Pulled latest changes.

% ./makecatalogs 
Using repo path: /Volumes/repo
Traceback (most recent call last):
  File "./makecatalogs", line 429, in <module>
    main()
  File "./makecatalogs", line 420, in main
    repo = Repo.Open(repopath, options.repo_url, options.plugin)
  File "/Users/Shared/ryanyu91/munki/code/client/munkilib/Repo.py", line 39, in Open
    module = imp.load_source('FileRepo', commandPath+'/munkilib/FileRepo.py')
IOError: [Errno 2] No such file or directory
@gregneagle

This comment has been minimized.

Show comment
Hide comment
@gregneagle

gregneagle Jan 18, 2017

Contributor
$ ./munkiimport /path/to/some.pkg 
Traceback (most recent call last):
  File "./munkiimport", line 1236, in <module>
    main()
  File "./munkiimport", line 981, in main
    repo = Repo.Open(REPO_PATH, REPO_URL, REPO_PLUGIN)
  File "/Users/Shared/ryanyu91/munki/code/client/munkilib/Repo.py", line 39, in Open
    module = imp.load_source('FileRepo', commandPath+'/munkilib/FileRepo.py')
IOError: [Errno 2] No such file or directory

Probably same core issue as the previous comment.

Contributor

gregneagle commented Jan 18, 2017

$ ./munkiimport /path/to/some.pkg 
Traceback (most recent call last):
  File "./munkiimport", line 1236, in <module>
    main()
  File "./munkiimport", line 981, in main
    repo = Repo.Open(REPO_PATH, REPO_URL, REPO_PLUGIN)
  File "/Users/Shared/ryanyu91/munki/code/client/munkilib/Repo.py", line 39, in Open
    module = imp.load_source('FileRepo', commandPath+'/munkilib/FileRepo.py')
IOError: [Errno 2] No such file or directory

Probably same core issue as the previous comment.

code/client/munkilib/FileRepo.py
+ #
+ def open(self, path, mode='r'):
+ '''Opens a file in the repo.'''
+ class RepoFile:

This comment has been minimized.

@gregneagle

gregneagle Jan 18, 2017

Contributor

Another "old-style" class. Should probably be class RepoFile(object):

@gregneagle

gregneagle Jan 18, 2017

Contributor

Another "old-style" class. Should probably be class RepoFile(object):

code/client/munkilib/iconutils.py
@@ -213,7 +230,7 @@ def extractAppIconsFromBundlePkg(pkg_path):
return icon_paths
-def getAppInfoPathsFromBundleComponentPkg(pkg_path):
+def getAppInfoPathsFromBOM(bomfile):
'''Returns a list of paths to application Info.plists'''
bomfile = os.path.join(pkg_path, u'Contents/Archive.bom')

This comment has been minimized.

@gregneagle

gregneagle Jan 18, 2017

Contributor

pkg_path is undefined here. probably this line just needs to go away since bomfile is passed to the function.

@gregneagle

gregneagle Jan 18, 2017

Contributor

pkg_path is undefined here. probably this line just needs to go away since bomfile is passed to the function.

@gregneagle

See line-by-line comments and the two tracebacks.

ryanyu91 added some commits Jan 16, 2017

Ammending last commit: 77b7c95
NEW CHANGES
************************************************************************************************************************************************************************************
#689 (review)
Commit based on comments by Greg Neagle on what to fix: munki#689 (master...ryanyu91:master)

FileRepo:
Changed FileRepo class definition to not be old-style class

iconutils:
Deleted pkg_path join since passing in file anyways

munkiimport:
for --configure option, changed for relative paths support from same directory

Repo:
Changed for relative paths support from same directory

************************************************************************************************************************************************************************************
#689 (review)
Commit based on comments by Greg Neagle on what to fix: munki#689 (master...ryanyu91:master)

makecatalogs:
removed debugging print statements

manifestutil:
check if repo is initialized before checking if it is mounted

munkiimport:
check if repo is initialized before checking if it is mounted
for def add_icon_hash_to_pkginfo(pkginfo):, not opening file (icon_path) anymore
WE_MOUNTED_THE_REPO is a repo attribute now, instead of a global variable

FileRepo:
removed duplicate/unused imports
changed class to not be old-style
updated glob method since pkgs was uninitialized
made WE_MOUNTED_THE_REPO from global variable to FileRepo attribute variable initialized to False

iconimporter:
updated Repo call to include 3 parameters now..
changed old find_items_to_check calls to new call: findItemsToCheck

Repo.py
Added support for relative paths for importing plugins
************************************************************************************************************************************************************************************

Commit based on comments by Greg Neagle on what to fix: munki#685 (master...ryanyu91:master)
IconImporter:

Not opening DMG and then mounting it anymore, directly mounting like how it was before
ManifestUtil:
Checking if the repo is mounted as well as if we (munki) mounted it. Only this will display prompt whether we want to unmount or not
FileRepo.py, Repo.py:
not hardcoding import path anymore
FileRepo - added 10.12 mounting fileshares code
************************************************************************************************************************************************************************************

Changed Files:
code/client/munkiimport
code/client/iconimporter
code/client/makecatalogs
code/client/manifestutil
code/client/munkilib/iconutils.py

Added Files:
code/client/munkilib/FileRepo.py
code/client/munkilib/Repo.py

Reason For Changes:

The purpose for this change is to enable plugins to munki that will allow writes to the munki repo to be customized.

Changes:

The methods used to write to the munki repo are the target of this modification. A plugin can create new methods for overwriting the default behavior of writing to the munki repo.

The default behavior is retained in the absence of a plugin. This is accomplished via the introduction of FileRepo.py which continues to simply write changes to the munki repo. A plugin can be introduced to change this default behavior and allow munki repo writes to be redirected as desired by the plugin author.

In order to accomplish this we have refactored all of the os.path.* methods, as well as the mount, unmount and available methods from the following tools: (iconimporter, makecatalogs, munkiimport, manifestutil, iconutils), and put them into a different python script to be used as a library or common code module. By default, these are now in the FileRepo.py module. The plugin can be used to overwrite this common code module.

In addition to the above described changes, we also made the following changes to support this plugin
concept:
•	Added the ability to add a plugin either via the command line or via munkiimport –configure
•	The code will look for custom plugins in the /usr/local/munki/munkilib/plugins/ directory
•	If plugin is found in the plugins directory, munkiimport --configure will give the option to type in the plugin name. For example, if you plugin is Foo.py, you could specify a plugin name of Foo in munkiimport --configure
•	If no plugin is found the FileRepo.py module will be used as the default common code module for writing to the munki repo
•	Add the ability to set the plugin via the --plugin option for the following tools (makecatalogs, munkiimport, manifestutil)

Testing
Tested with munkiimport on local filesystem/network shares/our own custom plugins
Tested manifestutil on local filesystem/network
Verified through regression testing, all features of munkiimport, makecatalogs, iconimporter, manifestutil from before all still work (local filesystem/network shares)
Merge branch 'master' of https://github.com/ryanyu91/munki
# Conflicts:
#	code/client/munkilib/Repo.py
@gregneagle

This comment has been minimized.

Show comment
Hide comment
@gregneagle

gregneagle Jan 20, 2017

Contributor

So this is no longer triggering the previous errors, but I'll need more testing time.

Any thoughts about maybe moving the FileRepo.py into the plugins folder and eliminating all the special-casing around that plugin?

IOW, just have plugin default to "FileRepo" and then use the exact same logic for finding it as you would any other repo plugin...

Contributor

gregneagle commented Jan 20, 2017

So this is no longer triggering the previous errors, but I'll need more testing time.

Any thoughts about maybe moving the FileRepo.py into the plugins folder and eliminating all the special-casing around that plugin?

IOW, just have plugin default to "FileRepo" and then use the exact same logic for finding it as you would any other repo plugin...

@ryanyu91

This comment has been minimized.

Show comment
Hide comment
@ryanyu91

ryanyu91 Jan 24, 2017

Contributor

Hi Greg,
That is probably a good idea. I'll commit again with the changes. It should be a small change.
Thanks!

Contributor

ryanyu91 commented Jan 24, 2017

Hi Greg,
That is probably a good idea. I'll commit again with the changes. It should be a small change.
Thanks!

NEW CHANGES
************************************************************************************************************************************************************************************
#689 (review)
Commit based on comments by Greg Neagle on what to fix: munki#689 (master...ryanyu91:master)

FileRepo:
Move FileRepo to munkilib/plugins/

Repo:
Change plugins location for FileRepo

munkiimport:
Change plugins location for FileRepo in configure.

************************************************************************************************************************************************************************************
#689 (review)
Commit based on comments by Greg Neagle on what to fix: munki#689 (master...ryanyu91:master)

FileRepo:
Changed FileRepo class definition to not be old-style class

iconutils:
Deleted pkg_path join since passing in file anyways

munkiimport:
for --configure option, changed for relative paths support from same directory

Repo:
Changed for relative paths support from same directory

************************************************************************************************************************************************************************************
#689 (review)
Commit based on comments by Greg Neagle on what to fix: munki#689 (master...ryanyu91:master)

makecatalogs:
removed debugging print statements

manifestutil:
check if repo is initialized before checking if it is mounted

munkiimport:
check if repo is initialized before checking if it is mounted
for def add_icon_hash_to_pkginfo(pkginfo):, not opening file (icon_path) anymore
WE_MOUNTED_THE_REPO is a repo attribute now, instead of a global variable

FileRepo:
removed duplicate/unused imports
changed class to not be old-style
updated glob method since pkgs was uninitialized
made WE_MOUNTED_THE_REPO from global variable to FileRepo attribute variable initialized to False

iconimporter:
updated Repo call to include 3 parameters now..
changed old find_items_to_check calls to new call: findItemsToCheck

Repo.py
Added support for relative paths for importing plugins
************************************************************************************************************************************************************************************

Commit based on comments by Greg Neagle on what to fix: munki#685 (master...ryanyu91:master)
IconImporter:

Not opening DMG and then mounting it anymore, directly mounting like how it was before
ManifestUtil:
Checking if the repo is mounted as well as if we (munki) mounted it. Only this will display prompt whether we want to unmount or not
FileRepo.py, Repo.py:
not hardcoding import path anymore
FileRepo - added 10.12 mounting fileshares code
************************************************************************************************************************************************************************************

Changed Files:
code/client/munkiimport
code/client/iconimporter
code/client/makecatalogs
code/client/manifestutil
code/client/munkilib/iconutils.py

Added Files:
code/client/munkilib/FileRepo.py
code/client/munkilib/Repo.py

Reason For Changes:

The purpose for this change is to enable plugins to munki that will allow writes to the munki repo to be customized.

Changes:

The methods used to write to the munki repo are the target of this modification. A plugin can create new methods for overwriting the default behavior of writing to the munki repo.

The default behavior is retained in the absence of a plugin. This is accomplished via the introduction of FileRepo.py which continues to simply write changes to the munki repo. A plugin can be introduced to change this default behavior and allow munki repo writes to be redirected as desired by the plugin author.

In order to accomplish this we have refactored all of the os.path.* methods, as well as the mount, unmount and available methods from the following tools: (iconimporter, makecatalogs, munkiimport, manifestutil, iconutils), and put them into a different python script to be used as a library or common code module. By default, these are now in the FileRepo.py module. The plugin can be used to overwrite this common code module.

In addition to the above described changes, we also made the following changes to support this plugin
concept:
•	Added the ability to add a plugin either via the command line or via munkiimport –configure
•	The code will look for custom plugins in the /usr/local/munki/munkilib/plugins/ directory
•	If plugin is found in the plugins directory, munkiimport --configure will give the option to type in the plugin name. For example, if you plugin is Foo.py, you could specify a plugin name of Foo in munkiimport --configure
•	If no plugin is found the FileRepo.py module will be used as the default common code module for writing to the munki repo
•	Add the ability to set the plugin via the --plugin option for the following tools (makecatalogs, munkiimport, manifestutil)

Testing
Tested with munkiimport on local filesystem/network shares/our own custom plugins
Tested manifestutil on local filesystem/network
Verified through regression testing, all features of munkiimport, makecatalogs, iconimporter, manifestutil from before all still work (local filesystem/network shares)
@gregneagle

Some more simplification

code/client/munkilib/Repo.py
+ module = imp.load_source(plugin, munkilib_path + '/plugins/' + plugin + ".py")
+ import_class = getattr(module, plugin)
+ parent = import_class
+

This comment has been minimized.

@gregneagle

gregneagle Jan 26, 2017

Contributor

Lines 46-54 should now be simplified to something like:

    if not plugin:
        plugin = 'FileRepo'
    plugin_path = os.path.join(munkilib_path, 'plugins', plugin + '.py')
    module = imp.load_source(plugin, plugin_path)
    import_class = getattr(module, plugin)
    parent = import_class
@gregneagle

gregneagle Jan 26, 2017

Contributor

Lines 46-54 should now be simplified to something like:

    if not plugin:
        plugin = 'FileRepo'
    plugin_path = os.path.join(munkilib_path, 'plugins', plugin + '.py')
    module = imp.load_source(plugin, plugin_path)
    import_class = getattr(module, plugin)
    parent = import_class
code/client/munkilib/iconutils.py
@@ -31,6 +31,7 @@
import munkicommon
import FoundationPlist
+import Repo

This comment has been minimized.

@gregneagle

gregneagle Jan 26, 2017

Contributor

This import is not actually used.

@gregneagle

gregneagle Jan 26, 2017

Contributor

This import is not actually used.

@gregneagle

This comment has been minimized.

Show comment
Hide comment
@gregneagle

gregneagle Jan 26, 2017

Contributor

More testing today. See interspersed comments:

% /Users/Shared/ryanyu91/munki/code/client/munkiimport DA_sshConfig/build/DA_sshConfig-1.8.pkg
Username: gneagle
Password: 

Compare this set of prompts to the current release:

    % munkiimport /Users/Shared/munki-git/munki-pkg/DA_sshConfig/build/DA_sshConfig-1.8.pkg
    Attempting to mount fileshare afp://munki/repo:
    Username: gneagle
    Password: 

Please put back the bit of text that tells you why we're asking for a Username and Password.
Back to the new code:

This item is similar to an existing item in the repo:
            Item name: DA_sshConfig
         Display name: Disney Animation SSH Config
          Description: Disney Animation secure shell (SSH) configuration
              Version: 1.7
  Installer item path: disney/DA_sshConfig-1.7.pkg

Use existing item as a template? [y/n] y
Copying unattended_install: True
Copying unattended_uninstall: True
Copying requires: (
    DAStartupScripts,
    DAperiodicConfig
)
Copying category: System Administration
Copying developer: Disney Animation
Copying icon_name: 
           Item name: DA_sshConfig
        Display name: Disney Animation SSH Config
         Description: Disney Animation secure shell (SSH) configuration
             Version: 1.8
            Category: System Administration
           Developer: Disney Animation
  Unattended install: True
Unattended uninstall: True
            Catalogs: production

Import this item? [y/n] y
No existing product icon found.
Attempt to create a product icon? [y/n] n
Copying DA_sshConfig-1.8.pkg to pkgs/disney/DA_sshConfig-1.8.pkg...
Saving pkginfo to pkgsinfo/disney/DA_sshConfig-1.8.plist...
The file /Users/Shared/munki-git/munki-pkg/pkgsinfo/disney/DA_sshConfig-1.8.plist does not exist.

That's weird. Why is it looking for that file there?

Problem running editor TextMate.app: Command '['/usr/bin/open', '-a', u'TextMate.app', u'pkgsinfo/disney/DA_sshConfig-1.8.plist']' returned non-zero exit status 1.

That fails since "pkgsinfo/disney/DA_sshConfig-1.8.plist" is a relative path.

Rebuild catalogs? [y/n] n
Unmount the repo fileshare? [y/n] y
Traceback (most recent call last):
  File "/Users/Shared/ryanyu91/munki/code/client/munkiimport", line 1250, in <module>
    main()
  File "/Users/Shared/ryanyu91/munki/code/client/munkiimport", line 1246, in main
    cleanup_and_exit(0)
  File "/Users/Shared/ryanyu91/munki/code/client/munkiimport", line 776, in cleanup_and_exit
    result = repo.unmount()
  File "/Users/Shared/ryanyu91/munki/code/client/munkilib/plugins/FileRepo.py", line 262, in unmount
    os.rmdir(self.path)
OSError: [Errno 2] No such file or directory: '/Volumes/repo'

So I'd say more work needs to happen to make sure that the new code doesn't break any existing functionality when people are using a file-based repo...

Contributor

gregneagle commented Jan 26, 2017

More testing today. See interspersed comments:

% /Users/Shared/ryanyu91/munki/code/client/munkiimport DA_sshConfig/build/DA_sshConfig-1.8.pkg
Username: gneagle
Password: 

Compare this set of prompts to the current release:

    % munkiimport /Users/Shared/munki-git/munki-pkg/DA_sshConfig/build/DA_sshConfig-1.8.pkg
    Attempting to mount fileshare afp://munki/repo:
    Username: gneagle
    Password: 

Please put back the bit of text that tells you why we're asking for a Username and Password.
Back to the new code:

This item is similar to an existing item in the repo:
            Item name: DA_sshConfig
         Display name: Disney Animation SSH Config
          Description: Disney Animation secure shell (SSH) configuration
              Version: 1.7
  Installer item path: disney/DA_sshConfig-1.7.pkg

Use existing item as a template? [y/n] y
Copying unattended_install: True
Copying unattended_uninstall: True
Copying requires: (
    DAStartupScripts,
    DAperiodicConfig
)
Copying category: System Administration
Copying developer: Disney Animation
Copying icon_name: 
           Item name: DA_sshConfig
        Display name: Disney Animation SSH Config
         Description: Disney Animation secure shell (SSH) configuration
             Version: 1.8
            Category: System Administration
           Developer: Disney Animation
  Unattended install: True
Unattended uninstall: True
            Catalogs: production

Import this item? [y/n] y
No existing product icon found.
Attempt to create a product icon? [y/n] n
Copying DA_sshConfig-1.8.pkg to pkgs/disney/DA_sshConfig-1.8.pkg...
Saving pkginfo to pkgsinfo/disney/DA_sshConfig-1.8.plist...
The file /Users/Shared/munki-git/munki-pkg/pkgsinfo/disney/DA_sshConfig-1.8.plist does not exist.

That's weird. Why is it looking for that file there?

Problem running editor TextMate.app: Command '['/usr/bin/open', '-a', u'TextMate.app', u'pkgsinfo/disney/DA_sshConfig-1.8.plist']' returned non-zero exit status 1.

That fails since "pkgsinfo/disney/DA_sshConfig-1.8.plist" is a relative path.

Rebuild catalogs? [y/n] n
Unmount the repo fileshare? [y/n] y
Traceback (most recent call last):
  File "/Users/Shared/ryanyu91/munki/code/client/munkiimport", line 1250, in <module>
    main()
  File "/Users/Shared/ryanyu91/munki/code/client/munkiimport", line 1246, in main
    cleanup_and_exit(0)
  File "/Users/Shared/ryanyu91/munki/code/client/munkiimport", line 776, in cleanup_and_exit
    result = repo.unmount()
  File "/Users/Shared/ryanyu91/munki/code/client/munkilib/plugins/FileRepo.py", line 262, in unmount
    os.rmdir(self.path)
OSError: [Errno 2] No such file or directory: '/Volumes/repo'

So I'd say more work needs to happen to make sure that the new code doesn't break any existing functionality when people are using a file-based repo...

@gregneagle

This comment has been minimized.

Show comment
Hide comment
@gregneagle

gregneagle Jan 26, 2017

Contributor

To summarize things in the previous comment -- there are at least three issues to address there:

  1. Fixing the prompt when mounting the repo -- asking for a username and password with no context is no bueno.

  2. Handing off additional editing of the pkginfo to a text editor. You may decide to not support that for Repos other than FileRepo, but you can't break the functionality for FileRepos.

  3. Crash at the end when unmounting the repo.

Contributor

gregneagle commented Jan 26, 2017

To summarize things in the previous comment -- there are at least three issues to address there:

  1. Fixing the prompt when mounting the repo -- asking for a username and password with no context is no bueno.

  2. Handing off additional editing of the pkginfo to a text editor. You may decide to not support that for Repos other than FileRepo, but you can't break the functionality for FileRepos.

  3. Crash at the end when unmounting the repo.

@gregneagle

This comment has been minimized.

Show comment
Hide comment
@gregneagle

gregneagle Feb 2, 2017

Contributor

Any updates?

Contributor

gregneagle commented Feb 2, 2017

Any updates?

@ryanyu91

This comment has been minimized.

Show comment
Hide comment
@ryanyu91

ryanyu91 Feb 2, 2017

Contributor

Hi Greg,
Sorry for the delay, been a busy week. Just finishing up the changes. :)

Contributor

ryanyu91 commented Feb 2, 2017

Hi Greg,
Sorry for the delay, been a busy week. Just finishing up the changes. :)

@gregneagle

This comment has been minimized.

Show comment
Hide comment
@gregneagle

gregneagle Feb 2, 2017

Contributor

No problem. Just wanted to make sure you weren't thinking the ball was in my court.

Contributor

gregneagle commented Feb 2, 2017

No problem. Just wanted to make sure you weren't thinking the ball was in my court.

NEW CHANGES
************************************************************************************************************************************************************************************
#689 (review)
Commit based on comments by Greg Neagle on what to fix: munki#689 (master...ryanyu91:master)

FileRepo:
Fixed crashing issue for unmounting.

iconutils:
removed unused import

Repo:
Simplified plugins selection based on plugins option.

Couldn’t reproduce the case where pkginfo failed to open in the textEditor. For me, the pkginfo always opened up in sublime (my pref setting for textedit or) Could you check to see if it is still happening with your repo? Thanks.

************************************************************************************************************************************************************************************
#689 (review)
Commit based on comments by Greg Neagle on what to fix: munki#689 (master...ryanyu91:master)

FileRepo:
Move FileRepo to munkilib/plugins/

Repo:
Change plugins location for FileRepo

munkiimport:
Change plugins location for FileRepo in configure.

************************************************************************************************************************************************************************************
#689 (review)
Commit based on comments by Greg Neagle on what to fix: munki#689 (master...ryanyu91:master)

FileRepo:
Changed FileRepo class definition to not be old-style class

iconutils:
Deleted pkg_path join since passing in file anyways

munkiimport:
for --configure option, changed for relative paths support from same directory

Repo:
Changed for relative paths support from same directory

************************************************************************************************************************************************************************************
#689 (review)
Commit based on comments by Greg Neagle on what to fix: munki#689 (master...ryanyu91:master)

makecatalogs:
removed debugging print statements

manifestutil:
check if repo is initialized before checking if it is mounted

munkiimport:
check if repo is initialized before checking if it is mounted
for def add_icon_hash_to_pkginfo(pkginfo):, not opening file (icon_path) anymore
WE_MOUNTED_THE_REPO is a repo attribute now, instead of a global variable

FileRepo:
removed duplicate/unused imports
changed class to not be old-style
updated glob method since pkgs was uninitialized
made WE_MOUNTED_THE_REPO from global variable to FileRepo attribute variable initialized to False

iconimporter:
updated Repo call to include 3 parameters now..
changed old find_items_to_check calls to new call: findItemsToCheck

Repo.py
Added support for relative paths for importing plugins
************************************************************************************************************************************************************************************

Commit based on comments by Greg Neagle on what to fix: munki#685 (master...ryanyu91:master)
IconImporter:

Not opening DMG and then mounting it anymore, directly mounting like how it was before
ManifestUtil:
Checking if the repo is mounted as well as if we (munki) mounted it. Only this will display prompt whether we want to unmount or not
FileRepo.py, Repo.py:
not hardcoding import path anymore
FileRepo - added 10.12 mounting fileshares code
************************************************************************************************************************************************************************************

Changed Files:
code/client/munkiimport
code/client/iconimporter
code/client/makecatalogs
code/client/manifestutil
code/client/munkilib/iconutils.py

Added Files:
code/client/munkilib/FileRepo.py
code/client/munkilib/Repo.py

Reason For Changes:

The purpose for this change is to enable plugins to munki that will allow writes to the munki repo to be customized.

Changes:

The methods used to write to the munki repo are the target of this modification. A plugin can create new methods for overwriting the default behavior of writing to the munki repo.

The default behavior is retained in the absence of a plugin. This is accomplished via the introduction of FileRepo.py which continues to simply write changes to the munki repo. A plugin can be introduced to change this default behavior and allow munki repo writes to be redirected as desired by the plugin author.

In order to accomplish this we have refactored all of the os.path.* methods, as well as the mount, unmount and available methods from the following tools: (iconimporter, makecatalogs, munkiimport, manifestutil, iconutils), and put them into a different python script to be used as a library or common code module. By default, these are now in the FileRepo.py module. The plugin can be used to overwrite this common code module.

In addition to the above described changes, we also made the following changes to support this plugin
concept:
•	Added the ability to add a plugin either via the command line or via munkiimport –configure
•	The code will look for custom plugins in the /usr/local/munki/munkilib/plugins/ directory
•	If plugin is found in the plugins directory, munkiimport --configure will give the option to type in the plugin name. For example, if you plugin is Foo.py, you could specify a plugin name of Foo in munkiimport --configure
•	If no plugin is found the FileRepo.py module will be used as the default common code module for writing to the munki repo
•	Add the ability to set the plugin via the --plugin option for the following tools (makecatalogs, munkiimport, manifestutil)

Testing
Tested with munkiimport on local filesystem/network shares/our own custom plugins
Tested manifestutil on local filesystem/network
Verified through regression testing, all features of munkiimport, makecatalogs, iconimporter, manifestutil from before all still work (local filesystem/network shares)
@gregneagle

This comment has been minimized.

Show comment
Hide comment
@gregneagle

gregneagle Feb 2, 2017

Contributor
Copying DA_sshConfig-1.9.pkg to pkgs/disney/DA_sshConfig-1.9__1.pkg...
Saving pkginfo to pkgsinfo/disney/DA_sshConfig-1.9__1.plist...
The file /Users/Shared/ryanyu91/munki/code/client/pkgsinfo/disney/DA_sshConfig-1.9__1.plist does not exist.
Problem running editor TextMate.app: Command '['/usr/bin/open', '-a', u'TextMate.app', u'pkgsinfo/disney/DA_sshConfig-1.9__1.plist']' returned non-zero exit status 1.

Text editor problem still exists. The crux of the problem is that it's passing the (repo) relative path to the open command, when it should be passing the absolute path.

IOW:
open -a TextMate.app pkgsinfo/disney/DA_sshConfig-1.9__1.plist
instead of
open -a TextMate.app /Volumes/repo/pkgsinfo/disney/DA_sshConfig-1.9__1.plist

Contributor

gregneagle commented Feb 2, 2017

Copying DA_sshConfig-1.9.pkg to pkgs/disney/DA_sshConfig-1.9__1.pkg...
Saving pkginfo to pkgsinfo/disney/DA_sshConfig-1.9__1.plist...
The file /Users/Shared/ryanyu91/munki/code/client/pkgsinfo/disney/DA_sshConfig-1.9__1.plist does not exist.
Problem running editor TextMate.app: Command '['/usr/bin/open', '-a', u'TextMate.app', u'pkgsinfo/disney/DA_sshConfig-1.9__1.plist']' returned non-zero exit status 1.

Text editor problem still exists. The crux of the problem is that it's passing the (repo) relative path to the open command, when it should be passing the absolute path.

IOW:
open -a TextMate.app pkgsinfo/disney/DA_sshConfig-1.9__1.plist
instead of
open -a TextMate.app /Volumes/repo/pkgsinfo/disney/DA_sshConfig-1.9__1.plist

@gregneagle

This comment has been minimized.

Show comment
Hide comment
@gregneagle

gregneagle Feb 3, 2017

Contributor

Confirmed latest commit fixes issue with opening the pkginfo in a text editor.

Contributor

gregneagle commented Feb 3, 2017

Confirmed latest commit fixes issue with opening the pkginfo in a text editor.

code/client/munkiimport
@@ -438,7 +438,7 @@ def copy_pkginfo_to_repo(pkginfo, subdirectory=''):
"""Saves pkginfo to munki_repo_path/pkgsinfo/subdirectory"""
# less error checking because we copy the installer_item
# first and bail if it fails...
- destination_path = repo.join('pkgsinfo', subdirectory)
+ destination_path = repo.join(REPO_PATH, 'pkgsinfo', subdirectory)

This comment has been minimized.

@gregneagle

gregneagle Feb 3, 2017

Contributor

This does fix the issue, though shouldn't maybe the repo keep track of its own path?

@gregneagle

gregneagle Feb 3, 2017

Contributor

This does fix the issue, though shouldn't maybe the repo keep track of its own path?

@gregneagle gregneagle referenced this pull request Feb 3, 2017

Closed

Centrify repo backend #616

@gregneagle

This comment has been minimized.

Show comment
Hide comment
@gregneagle

gregneagle Feb 14, 2017

Contributor

I've merged this into munki/cloudrepo here: https://github.com/munki/munki/tree/cloudrepo and will be asking for community testing and feedback prior to it being merged into master.

Contributor

gregneagle commented Feb 14, 2017

I've merged this into munki/cloudrepo here: https://github.com/munki/munki/tree/cloudrepo and will be asking for community testing and feedback prior to it being merged into master.

Check plugin path exists before utilizing it so munkiimport —configur…
…e will not crash if plugins folder does not exist
@gregneagle

This comment has been minimized.

Show comment
Hide comment
@gregneagle

gregneagle Feb 22, 2017

Contributor

I thought the FileRepo plugin had moved to the plugin_path, so if the plug_in path is missing, we've got big, big problems and should exit with an error message.

I thought the FileRepo plugin had moved to the plugin_path, so if the plug_in path is missing, we've got big, big problems and should exit with an error message.

@ryanyu91

This comment has been minimized.

Show comment
Hide comment
@ryanyu91

ryanyu91 Feb 22, 2017

Contributor

Yes, FileRepo has moved to plugins folder, should we revert that fix then?

Contributor

ryanyu91 commented Feb 22, 2017

Yes, FileRepo has moved to plugins folder, should we revert that fix then?

@gregneagle

This comment has been minimized.

Show comment
Hide comment
@gregneagle

gregneagle Feb 22, 2017

Contributor

Test this: remove the plugins folder and then attempt to run the affected tools. Modify the behavior to maximize helpfulness.

Contributor

gregneagle commented Feb 22, 2017

Test this: remove the plugins folder and then attempt to run the affected tools. Modify the behavior to maximize helpfulness.

@ryanyu91

This comment has been minimized.

Show comment
Hide comment
@ryanyu91

ryanyu91 Feb 27, 2017

Contributor

I agree we should get the error message in munkiimport configure too. This way the user is not confused as to why munkiimport configure completed with no errors but munkiimport will if plugins folder is missing. I will revert this commit.

Contributor

ryanyu91 commented Feb 27, 2017

I agree we should get the error message in munkiimport configure too. This way the user is not confused as to why munkiimport configure completed with no errors but munkiimport will if plugins folder is missing. I will revert this commit.

Revert "Check plugin path exists before utilizing it so munkiimport —…
…configure will not crash if plugins folder does not exist"

This reverts commit 920e6b1.
@gregneagle

This comment has been minimized.

Show comment
Hide comment
@gregneagle

gregneagle Mar 2, 2017

Contributor

The cloudrepo branch has been merged into the Munki3 branch and is on track for a future release. For future development/testing, please base any future changes on the code currently in the Munki3 branch.

Contributor

gregneagle commented Mar 2, 2017

The cloudrepo branch has been merged into the Munki3 branch and is on track for a future release. For future development/testing, please base any future changes on the code currently in the Munki3 branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment