Gentoo Archives: gentoo-portage-dev

From: "Michał Górny" <mgorny@g.o>
To: Alexander Berntsen <bernalex@g.o>
Cc: gentoo-portage-dev@l.g.o
Subject: Re: [gentoo-portage-dev] How do we want to deal with Arfrever's bunch of patches?
Date: Sat, 12 Dec 2015 14:50:53
Message-Id: 20151212155035.4ed56b58.mgorny@gentoo.org
In Reply to: [gentoo-portage-dev] How do we want to deal with Arfrever's bunch of patches? by Alexander Berntsen
1 On Sat, 12 Dec 2015 13:01:04 +0100
2 Alexander Berntsen <bernalex@g.o> wrote:
3
4 > Friends,
5 >
6 > As I'm sure you've noticed, Arfrever pushed a bunch of unreviewed
7 > stuff. Michał reverted some of them as they were breaking changes in
8 > addition to being unreviewed (oh, and of course they don't follow the
9 > commit message format -- why would they). There's a bunch left.
10 >
11 > Do we want to cherry-pick Zac's patches (i.e. revert Arfrever's
12 > patches), and have him put them up for review as usual? It's a bit of
13 > a mess because of Zac's patches. If we go down this route it would
14 > likely be easiest for Zac to revert them, since he'll be the more
15 > confident regarding what to keep in the ensuing merge conflicts.
16 >
17 > Arfrever asked that someone review the patches and keep them if they
18 > are OK, and revert only the ones that are problematic. I think this
19 > would be OK for now, but obviously not something we want to encourage
20 > or allow in the future.
21
22 Here's my quick review of the remaining commits (newest first):
23
24
25 31923f4 portage.package.ebuild.config.config.__init__(): Skip some warnings for Portage Python scripts called in ebuild environment.
26 7853950 Delete support for PORTDIR and PORTDIR_OVERLAY from make.conf and environment.
27 e7d95cb portage.repository.config.RepoConfig: Support location with trailing whitespace by using quoting.
28
29 Those three already reverted.
30
31
32 8036223 _emerge.depgraph.depgraph._select_files(): Update message.
33
34 Can probably be left, it's updating messages for repos.conf which is still valid.
35
36
37 9716f8a emirrordist: Delete support for deprecated --portdir and --portdir-overlay options.
38 2d40eb4 egencache: Delete support for deprecated --portdir and --portdir-overlay options.
39
40 Not sure. Maybe something relies on this.
41
42
43 fb4d1f4 ebuild: Rename some variables.
44
45 I'd keep it, doesn't hurt anything and the new names are more readable than 'myrepo' ;-).
46
47
48 9e104c4 ebuild: Set PORTAGE_REPOSITORIES instead of deprecated PORTDIR_OVERLAY.
49
50 Kill it as relying on weirdo PORTAGE_REPOSITORIES invention.
51
52
53 3917544 bin/socks5-server.py: Fix DeprecationWarning with Python >=3.4.4.
54
55 This one looks good, and even the commit message is sane.
56
57
58 2cde1f6 portage.repository.config: Clean reading of repository names and drop support for repositories with missing or invalid names.
59
60 I'd revert it, it's a breaking change.
61
62
63 1064765 portage.repository.config.RepoConfig: Delete user_location attribute and consistently use location attribute everywhere.
64
65 I think we can keep this if it doesn't break anything. I didn't hear
66 about this attribute before, so I suggest Zac reviews this.
67
68
69 48c2af4 Respect PYTHONDONTWRITEBYTECODE in test suite.
70
71 I don't mind either way. But can't we make this simpler?
72
73
74 528dc7c portage.package.ebuild.fetch.fetch(): Clean setting of variables.
75
76 Purely stylistic change.
77
78
79 10cccf7 Support environmental variables overriding parts of configuration of repositories.
80
81 Kill this ugly thing with a lot of fire. This is so wrong you can't get
82 much worse.
83
84
85 39d81c5 portage.package.ebuild.config.config.__getitem__(): Partially drop backward compatibility for nonexistent keys.
86
87 Maybe keep it but needs someone smarter than me to review.
88
89
90 4f25fa6 ebuild: Move imports to the top.
91 5ea789a portage.package.ebuild._config.LicenseManager.LicenseManager._getPkgAcceptLicense(): Fix typo in call to hasattr().
92
93 Makes sense.
94
95
96 3ffdbbe ebuild: Do not catch unexpected KeyErrors from aux_get().
97
98 Maybe. Need others to confirm.
99
100
101 b8da04b portageq envvar: Return 1 for any nonexistent variable.
102
103 Makes sense.
104
105
106 53a0b63 _emerge.actions.getgccversion(): Work with chost=None and with no explicit chost.
107
108 Probably makes sense, needs extra review.
109
110
111 628aa10 portage.sync.modules.cvs.CheckCVSConfig.check_cvs_repo(): Fix "KeyError: 'sync-cvs-repo'".
112
113 Makes sense.
114
115
116 f05056c portage.repository.config.RepoConfigLoader._parse(): Delete unused portdir parametre.
117 041fc70 portage.repository.config.RepoConfigLoader._add_repositories(): Delete unused ignored_location_map parametre.
118 c1a2714 portage.repository.config.RepoConfigLoader._parse(): Delete unused ignored_map and ignored_location_map parametres.
119
120 Those all make sense.
121
122 --
123 Best regards,
124 Michał Górny
125 <http://dev.gentoo.org/~mgorny/>

Replies