{"action":"created","issue":{"url":"https://api.github.com/repos/SUSE/kiwi/issues/191","repository_url":"https://api.github.com/repos/SUSE/kiwi","labels_url":"https://api.github.com/repos/SUSE/kiwi/issues/191/labels{/name}","comments_url":"https://api.github.com/repos/SUSE/kiwi/issues/191/comments","events_url":"https://api.github.com/repos/SUSE/kiwi/issues/191/events","html_url":"https://github.com/SUSE/kiwi/pull/191","id":188217624,"number":191,"title":"Clean all repositories after using PackageManager","user":{"login":"davidcassany","id":6428768,"avatar_url":"https://avatars.githubusercontent.com/u/6428768?v=3","gravatar_id":"","url":"https://api.github.com/users/davidcassany","html_url":"https://github.com/davidcassany","followers_url":"https://api.github.com/users/davidcassany/followers","following_url":"https://api.github.com/users/davidcassany/following{/other_user}","gists_url":"https://api.github.com/users/davidcassany/gists{/gist_id}","starred_url":"https://api.github.com/users/davidcassany/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/davidcassany/subscriptions","organizations_url":"https://api.github.com/users/davidcassany/orgs","repos_url":"https://api.github.com/users/davidcassany/repos","events_url":"https://api.github.com/users/davidcassany/events{/privacy}","received_events_url":"https://api.github.com/users/davidcassany/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2016-11-09T10:51:38Z","updated_at":"2016-11-09T15:10:25Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/SUSE/kiwi/pulls/191","html_url":"https://github.com/SUSE/kiwi/pull/191","diff_url":"https://github.com/SUSE/kiwi/pull/191.diff","patch_url":"https://github.com/SUSE/kiwi/pull/191.patch"},"body":"Running `repo.delete_all_repos()` in SystemSetup can tricky, as it happens after the `root` files provided by the user are copied in the build-root tree. Using zypper this repository cleaning means to delete anything inside `/etc/zypp/repos.d/` which ends up in deleting any file that the user could have added including it inside its own `root/etc/zypp/repos.d`. This might be case if trying to include repositories in the resulting image in OBS builds.\r\n\r\nThe proposed patch removes that `repo.delete_all_repos()` call from SystemSetup and includes it inside PackageManager instance destructor. PackageManager instances are used by SystemPrepare in order to build the image root tree before appending user defined files. This way the build tree is clean of repositories before appending user defined files, then repositories can be added in SystemSetup and/or by placing configuration files in the `root` folder.\r\n\r\nThis PR fixes #182."},"comment":{"url":"https://api.github.com/repos/SUSE/kiwi/issues/comments/259436814","html_url":"https://github.com/SUSE/kiwi/pull/191#issuecomment-259436814","issue_url":"https://api.github.com/repos/SUSE/kiwi/issues/191","id":259436814,"user":{"login":"schaefi","id":912234,"avatar_url":"https://avatars.githubusercontent.com/u/912234?v=3","gravatar_id":"","url":"https://api.github.com/users/schaefi","html_url":"https://github.com/schaefi","followers_url":"https://api.github.com/users/schaefi/followers","following_url":"https://api.github.com/users/schaefi/following{/other_user}","gists_url":"https://api.github.com/users/schaefi/gists{/gist_id}","starred_url":"https://api.github.com/users/schaefi/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/schaefi/subscriptions","organizations_url":"https://api.github.com/users/schaefi/orgs","repos_url":"https://api.github.com/users/schaefi/repos","events_url":"https://api.github.com/users/schaefi/events{/privacy}","received_events_url":"https://api.github.com/users/schaefi/received_events","type":"User","site_admin":false},"created_at":"2016-11-09T15:10:25Z","updated_at":"2016-11-09T15:10:25Z","body":"I'm a bit concerned, most probably because I don't understand the problem.\r\n\r\nThe repository interface defines among others the three methods\r\n\r\n* add_repo\r\n* delete_repo\r\n* delete_all_repos\r\n\r\n- As they belong to a RepositorySomething class one could argue that the short form add, delete and   delete_all would have been enough, ok\r\n\r\n- The delete_all just exists because repos are always a bunch of files in a directory and instead of itarating over the instances of RepositorySomething and call delete you have the delete_all joker on any of these instances. It's a nice to have thing to save two lines of iteration code... also questionable but I like it :)\r\n\r\nWith regards to the original issue, and I think this pull request should focus on getting that fixed first before mixing it with refactoring, I think your problem is that you can't add obs repos as imageinclude because we can't really resolve them into an external location\r\n\r\nActually the import_repositories_marked_as_imageinclude() method should only delete all repos if there are repos marked imageinclude in the image description. If no such repo definition exists it should not delete anything. This imho needs to be fixed.\r\n\r\nHowever _if_ there are repos added and marked as imageinclude we want the repo setup to consist exclusively out of these entries and nothing else. Therefore the method calls the delete_all\r\n\r\nWhen building in obs imageinclude doesn't make sense, so no repo should have that flag set\r\n\r\nLet's fix import_repositories_marked_as_imageinclude() first\r\n\r\nI don't want to have the repository destructor to delete all repos that is not the intended behavior\r\n"}}
{"actor":{"display_login":"schaefi"}}