{"action":"created","comment":{"url":"https://api.github.com/repos/puppetlabs/puppet/pulls/comments/88366675","pull_request_review_id":8942821,"id":88366675,"diff_hunk":"@@ -185,13 +185,24 @@ def validate(value)\n   end\n \n   # @api private\n+  # @param option [String] Extra file operation mode information to use\n+  #   (defaults to read-only mode 'r')\n+  #   This is the standard mechanism Ruby uses in the IO class, and therefore\n+  #   encoding may be explicitly like fmode : encoding or fmode : \"BOM|UTF-*\"\n+  #   for example, a:ASCII or w+:UTF-8\n   def exclusive_open(option = 'r', &block)\n     controlled_access do |mode|\n       Puppet::FileSystem.exclusive_open(file(), mode, option, &block)\n     end\n   end\n \n   # @api private\n+  # TODO: need to find callers of this guy","path":"lib/puppet/settings/file_setting.rb","position":16,"original_position":16,"commit_id":"b8acd69dbed29df3d971547f56cf07f6dd0de6be","original_commit_id":"b8acd69dbed29df3d971547f56cf07f6dd0de6be","user":{"login":"MosesMendoza","id":819133,"avatar_url":"https://avatars.githubusercontent.com/u/819133?v=3","gravatar_id":"","url":"https://api.github.com/users/MosesMendoza","html_url":"https://github.com/MosesMendoza","followers_url":"https://api.github.com/users/MosesMendoza/followers","following_url":"https://api.github.com/users/MosesMendoza/following{/other_user}","gists_url":"https://api.github.com/users/MosesMendoza/gists{/gist_id}","starred_url":"https://api.github.com/users/MosesMendoza/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/MosesMendoza/subscriptions","organizations_url":"https://api.github.com/users/MosesMendoza/orgs","repos_url":"https://api.github.com/users/MosesMendoza/repos","events_url":"https://api.github.com/users/MosesMendoza/events{/privacy}","received_events_url":"https://api.github.com/users/MosesMendoza/received_events","type":"User","site_admin":false},"body":"Was this `TODO` intentionally left in? ","created_at":"2016-11-17T00:58:35Z","updated_at":"2016-11-17T00:58:35Z","html_url":"https://github.com/puppetlabs/puppet/pull/5370#discussion_r88366675","pull_request_url":"https://api.github.com/repos/puppetlabs/puppet/pulls/5370","_links":{"self":{"href":"https://api.github.com/repos/puppetlabs/puppet/pulls/comments/88366675"},"html":{"href":"https://github.com/puppetlabs/puppet/pull/5370#discussion_r88366675"},"pull_request":{"href":"https://api.github.com/repos/puppetlabs/puppet/pulls/5370"}}},"pull_request":{"url":"https://api.github.com/repos/puppetlabs/puppet/pulls/5370","id":92835267,"html_url":"https://github.com/puppetlabs/puppet/pull/5370","diff_url":"https://github.com/puppetlabs/puppet/pull/5370.diff","patch_url":"https://github.com/puppetlabs/puppet/pull/5370.patch","issue_url":"https://api.github.com/repos/puppetlabs/puppet/issues/5370","number":5370,"state":"open","locked":false,"title":"(PUP-6188) Puppet::FileSystem and explicit Encoding changes","user":{"login":"Iristyle","id":394081,"avatar_url":"https://avatars.githubusercontent.com/u/394081?v=3","gravatar_id":"","url":"https://api.github.com/users/Iristyle","html_url":"https://github.com/Iristyle","followers_url":"https://api.github.com/users/Iristyle/followers","following_url":"https://api.github.com/users/Iristyle/following{/other_user}","gists_url":"https://api.github.com/users/Iristyle/gists{/gist_id}","starred_url":"https://api.github.com/users/Iristyle/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Iristyle/subscriptions","organizations_url":"https://api.github.com/users/Iristyle/orgs","repos_url":"https://api.github.com/users/Iristyle/repos","events_url":"https://api.github.com/users/Iristyle/events{/privacy}","received_events_url":"https://api.github.com/users/Iristyle/received_events","type":"User","site_admin":false},"body":"Supercedes #5354 \r\n\r\nChanneling @MosesMendoza from prior PR:\r\n\r\nThis PR builds on / supersedes #4898 from @Iristyle \r\n\r\n[PUP-6188](tickets.puppetlabs.com/browse/PUP-6188) tracks an effort to ensure Puppet's various file system interactions are specifying utf-8 encoding where possible/applicable, possibly in `Puppet::FileSystem.open` and `Puppet::FileSystem.exclusive_open`, but notes the problematic situation resulting from the reordering of method arguments from `Puppet::FileSystem::FileImpl#open` to [`::File.open`](https://ruby-doc.org/core-2.3.1/File.html#method-c-open):\r\n\r\n> - Rubys `perm` in our call is renamed as `mode`, and is moved from position 2 to position 1\r\n> - Rubys `mode` in our call is renamed as `options`, and is moved from position 1 to position 2\r\n> - We don't support the passing in of Rubys `opt` which is where one would set ``:encoding => 'utf-8'``\r\n\r\nThis is further complicated by the fact that ruby allows specifying mode, encoding, and other options a number of different ways via these parameters. For example, encoding and mode, which can be specified\r\n- via `opt` (ruby optional param 3) in an options hash, ie `:mode => 'r', :encoding => 'BOM|UTF-8'` \r\n- via a `mode` string (ruby param 2), containing one of:\r\n  - an IO Open Mode: `\"r\"`, `\"r+\"`, `\"w\"`\r\n  - an IO Open Mode _with encoding_: `\"r:IBM437\"`, `\"w:BOM|UTF-8\"`\r\n  - a list of `|` separated File Constants: `File::CREATE|File::APPEND`\r\n\r\nThis means it's not simply a matter of extending the definition of `Puppet::FileSystem::FileImpl#open` and `Puppet::FileSystem::FileImpl#exclusive_open` to support an options hash (and thus specifying `:encoding => 'UTF-8'` because such options may collide with those already specified by a user via the `mode` string (ruby param 2). We could probably handle this somewhat gracefully (a rudimentary first pass at this is in https://github.com/MosesMendoza/puppet/commit/ccb65927b22544239adbc49a7b054cc91016bdde) but it seems like less risk of backwards-incompatibility and less complexity overall to simply update the callers of `Puppet::FileSystem` inside of puppet to leverage the ability to pass encoding in via the `mode` string.\r\n\r\nThis PR does so in several places, explicitly specifying UTF-8 as part of the mode string and updating associated tests. When updating the `puppet config` face, it was discovered that the `:set` action has no unit testing associated with it (at least not that I can find) - all testing is at the lower level of `Puppet::Settings::Inifile`, so this PR also includes a commit that adds initial basic spec coverage of `puppet config set`. Additionally, where encountered, `File.read` was replaced with `Puppet::FileSystem.read` - it seemed as if some of these places existed before `Puppet::FileSystem` was implemented.\r\n\r\nAfter discussion with @Iristyle it was determined that much of the test coverage here likely deserves substantial improvement. Specifically, creating new tests that actually create files containing unicode-only characters, passing them through the code under test, and validating the fidelity of the data actually remains intact. I'm not sure if this PR should be gated on the addition of such tests, but I don't disagree that they're justified.\r\n","created_at":"2016-11-08T18:42:06Z","updated_at":"2016-11-17T00:58:35Z","closed_at":null,"merged_at":null,"merge_commit_sha":"223a5bf2a4738e58f04256858af1aa3ce7bffc40","assignee":null,"assignees":[],"milestone":null,"commits_url":"https://api.github.com/repos/puppetlabs/puppet/pulls/5370/commits","review_comments_url":"https://api.github.com/repos/puppetlabs/puppet/pulls/5370/comments","review_comment_url":"https://api.github.com/repos/puppetlabs/puppet/pulls/comments{/number}","comments_url":"https://api.github.com/repos/puppetlabs/puppet/issues/5370/comments","statuses_url":"https://api.github.com/repos/puppetlabs/puppet/statuses/b8acd69dbed29df3d971547f56cf07f6dd0de6be","head":{"label":"Iristyle:pr/5354","ref":"pr/5354","sha":"b8acd69dbed29df3d971547f56cf07f6dd0de6be","user":{"login":"Iristyle","id":394081,"avatar_url":"https://avatars.githubusercontent.com/u/394081?v=3","gravatar_id":"","url":"https://api.github.com/users/Iristyle","html_url":"https://github.com/Iristyle","followers_url":"https://api.github.com/users/Iristyle/followers","following_url":"https://api.github.com/users/Iristyle/following{/other_user}","gists_url":"https://api.github.com/users/Iristyle/gists{/gist_id}","starred_url":"https://api.github.com/users/Iristyle/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Iristyle/subscriptions","organizations_url":"https://api.github.com/users/Iristyle/orgs","repos_url":"https://api.github.com/users/Iristyle/repos","events_url":"https://api.github.com/users/Iristyle/events{/privacy}","received_events_url":"https://api.github.com/users/Iristyle/received_events","type":"User","site_admin":false},"repo":{"id":12253882,"name":"puppet","full_name":"Iristyle/puppet","owner":{"login":"Iristyle","id":394081,"avatar_url":"https://avatars.githubusercontent.com/u/394081?v=3","gravatar_id":"","url":"https://api.github.com/users/Iristyle","html_url":"https://github.com/Iristyle","followers_url":"https://api.github.com/users/Iristyle/followers","following_url":"https://api.github.com/users/Iristyle/following{/other_user}","gists_url":"https://api.github.com/users/Iristyle/gists{/gist_id}","starred_url":"https://api.github.com/users/Iristyle/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Iristyle/subscriptions","organizations_url":"https://api.github.com/users/Iristyle/orgs","repos_url":"https://api.github.com/users/Iristyle/repos","events_url":"https://api.github.com/users/Iristyle/events{/privacy}","received_events_url":"https://api.github.com/users/Iristyle/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/Iristyle/puppet","description":"Server automation framework and application","fork":true,"url":"https://api.github.com/repos/Iristyle/puppet","forks_url":"https://api.github.com/repos/Iristyle/puppet/forks","keys_url":"https://api.github.com/repos/Iristyle/puppet/keys{/key_id}","collaborators_url":"https://api.github.com/repos/Iristyle/puppet/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/Iristyle/puppet/teams","hooks_url":"https://api.github.com/repos/Iristyle/puppet/hooks","issue_events_url":"https://api.github.com/repos/Iristyle/puppet/issues/events{/number}","events_url":"https://api.github.com/repos/Iristyle/puppet/events","assignees_url":"https://api.github.com/repos/Iristyle/puppet/assignees{/user}","branches_url":"https://api.github.com/repos/Iristyle/puppet/branches{/branch}","tags_url":"https://api.github.com/repos/Iristyle/puppet/tags","blobs_url":"https://api.github.com/repos/Iristyle/puppet/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/Iristyle/puppet/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/Iristyle/puppet/git/refs{/sha}","trees_url":"https://api.github.com/repos/Iristyle/puppet/git/trees{/sha}","statuses_url":"https://api.github.com/repos/Iristyle/puppet/statuses/{sha}","languages_url":"https://api.github.com/repos/Iristyle/puppet/languages","stargazers_url":"https://api.github.com/repos/Iristyle/puppet/stargazers","contributors_url":"https://api.github.com/repos/Iristyle/puppet/contributors","subscribers_url":"https://api.github.com/repos/Iristyle/puppet/subscribers","subscription_url":"https://api.github.com/repos/Iristyle/puppet/subscription","commits_url":"https://api.github.com/repos/Iristyle/puppet/commits{/sha}","git_commits_url":"https://api.github.com/repos/Iristyle/puppet/git/commits{/sha}","comments_url":"https://api.github.com/repos/Iristyle/puppet/comments{/number}","issue_comment_url":"https://api.github.com/repos/Iristyle/puppet/issues/comments{/number}","contents_url":"https://api.github.com/repos/Iristyle/puppet/contents/{+path}","compare_url":"https://api.github.com/repos/Iristyle/puppet/compare/{base}...{head}","merges_url":"https://api.github.com/repos/Iristyle/puppet/merges","archive_url":"https://api.github.com/repos/Iristyle/puppet/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/Iristyle/puppet/downloads","issues_url":"https://api.github.com/repos/Iristyle/puppet/issues{/number}","pulls_url":"https://api.github.com/repos/Iristyle/puppet/pulls{/number}","milestones_url":"https://api.github.com/repos/Iristyle/puppet/milestones{/number}","notifications_url":"https://api.github.com/repos/Iristyle/puppet/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/Iristyle/puppet/labels{/name}","releases_url":"https://api.github.com/repos/Iristyle/puppet/releases{/id}","deployments_url":"https://api.github.com/repos/Iristyle/puppet/deployments","created_at":"2013-08-20T20:49:43Z","updated_at":"2015-02-14T00:06:23Z","pushed_at":"2016-11-16T16:36:53Z","git_url":"git://github.com/Iristyle/puppet.git","ssh_url":"git@github.com:Iristyle/puppet.git","clone_url":"https://github.com/Iristyle/puppet.git","svn_url":"https://github.com/Iristyle/puppet","homepage":"http://www.puppetlabs.com/puppet/introduction/","size":54149,"stargazers_count":0,"watchers_count":0,"language":"Ruby","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"puppetlabs:master","ref":"master","sha":"deae100aa0bb5be12790bc685b3e5ac936672c38","user":{"login":"puppetlabs","id":234268,"avatar_url":"https://avatars.githubusercontent.com/u/234268?v=3","gravatar_id":"","url":"https://api.github.com/users/puppetlabs","html_url":"https://github.com/puppetlabs","followers_url":"https://api.github.com/users/puppetlabs/followers","following_url":"https://api.github.com/users/puppetlabs/following{/other_user}","gists_url":"https://api.github.com/users/puppetlabs/gists{/gist_id}","starred_url":"https://api.github.com/users/puppetlabs/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/puppetlabs/subscriptions","organizations_url":"https://api.github.com/users/puppetlabs/orgs","repos_url":"https://api.github.com/users/puppetlabs/repos","events_url":"https://api.github.com/users/puppetlabs/events{/privacy}","received_events_url":"https://api.github.com/users/puppetlabs/received_events","type":"Organization","site_admin":false},"repo":{"id":910744,"name":"puppet","full_name":"puppetlabs/puppet","owner":{"login":"puppetlabs","id":234268,"avatar_url":"https://avatars.githubusercontent.com/u/234268?v=3","gravatar_id":"","url":"https://api.github.com/users/puppetlabs","html_url":"https://github.com/puppetlabs","followers_url":"https://api.github.com/users/puppetlabs/followers","following_url":"https://api.github.com/users/puppetlabs/following{/other_user}","gists_url":"https://api.github.com/users/puppetlabs/gists{/gist_id}","starred_url":"https://api.github.com/users/puppetlabs/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/puppetlabs/subscriptions","organizations_url":"https://api.github.com/users/puppetlabs/orgs","repos_url":"https://api.github.com/users/puppetlabs/repos","events_url":"https://api.github.com/users/puppetlabs/events{/privacy}","received_events_url":"https://api.github.com/users/puppetlabs/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/puppetlabs/puppet","description":"Server automation framework and application","fork":false,"url":"https://api.github.com/repos/puppetlabs/puppet","forks_url":"https://api.github.com/repos/puppetlabs/puppet/forks","keys_url":"https://api.github.com/repos/puppetlabs/puppet/keys{/key_id}","collaborators_url":"https://api.github.com/repos/puppetlabs/puppet/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/puppetlabs/puppet/teams","hooks_url":"https://api.github.com/repos/puppetlabs/puppet/hooks","issue_events_url":"https://api.github.com/repos/puppetlabs/puppet/issues/events{/number}","events_url":"https://api.github.com/repos/puppetlabs/puppet/events","assignees_url":"https://api.github.com/repos/puppetlabs/puppet/assignees{/user}","branches_url":"https://api.github.com/repos/puppetlabs/puppet/branches{/branch}","tags_url":"https://api.github.com/repos/puppetlabs/puppet/tags","blobs_url":"https://api.github.com/repos/puppetlabs/puppet/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/puppetlabs/puppet/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/puppetlabs/puppet/git/refs{/sha}","trees_url":"https://api.github.com/repos/puppetlabs/puppet/git/trees{/sha}","statuses_url":"https://api.github.com/repos/puppetlabs/puppet/statuses/{sha}","languages_url":"https://api.github.com/repos/puppetlabs/puppet/languages","stargazers_url":"https://api.github.com/repos/puppetlabs/puppet/stargazers","contributors_url":"https://api.github.com/repos/puppetlabs/puppet/contributors","subscribers_url":"https://api.github.com/repos/puppetlabs/puppet/subscribers","subscription_url":"https://api.github.com/repos/puppetlabs/puppet/subscription","commits_url":"https://api.github.com/repos/puppetlabs/puppet/commits{/sha}","git_commits_url":"https://api.github.com/repos/puppetlabs/puppet/git/commits{/sha}","comments_url":"https://api.github.com/repos/puppetlabs/puppet/comments{/number}","issue_comment_url":"https://api.github.com/repos/puppetlabs/puppet/issues/comments{/number}","contents_url":"https://api.github.com/repos/puppetlabs/puppet/contents/{+path}","compare_url":"https://api.github.com/repos/puppetlabs/puppet/compare/{base}...{head}","merges_url":"https://api.github.com/repos/puppetlabs/puppet/merges","archive_url":"https://api.github.com/repos/puppetlabs/puppet/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/puppetlabs/puppet/downloads","issues_url":"https://api.github.com/repos/puppetlabs/puppet/issues{/number}","pulls_url":"https://api.github.com/repos/puppetlabs/puppet/pulls{/number}","milestones_url":"https://api.github.com/repos/puppetlabs/puppet/milestones{/number}","notifications_url":"https://api.github.com/repos/puppetlabs/puppet/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/puppetlabs/puppet/labels{/name}","releases_url":"https://api.github.com/repos/puppetlabs/puppet/releases{/id}","deployments_url":"https://api.github.com/repos/puppetlabs/puppet/deployments","created_at":"2010-09-14T19:26:44Z","updated_at":"2016-11-15T15:31:08Z","pushed_at":"2016-11-16T22:23:51Z","git_url":"git://github.com/puppetlabs/puppet.git","ssh_url":"git@github.com:puppetlabs/puppet.git","clone_url":"https://github.com/puppetlabs/puppet.git","svn_url":"https://github.com/puppetlabs/puppet","homepage":"https://puppet.com/puppet/what-is-puppet","size":83692,"stargazers_count":4240,"watchers_count":4240,"language":"Ruby","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":1746,"mirror_url":null,"open_issues_count":62,"forks":1746,"open_issues":62,"watchers":4240,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/puppetlabs/puppet/pulls/5370"},"html":{"href":"https://github.com/puppetlabs/puppet/pull/5370"},"issue":{"href":"https://api.github.com/repos/puppetlabs/puppet/issues/5370"},"comments":{"href":"https://api.github.com/repos/puppetlabs/puppet/issues/5370/comments"},"review_comments":{"href":"https://api.github.com/repos/puppetlabs/puppet/pulls/5370/comments"},"review_comment":{"href":"https://api.github.com/repos/puppetlabs/puppet/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/puppetlabs/puppet/pulls/5370/commits"},"statuses":{"href":"https://api.github.com/repos/puppetlabs/puppet/statuses/b8acd69dbed29df3d971547f56cf07f6dd0de6be"}}}}
{"actor":{"display_login":"MosesMendoza"}}