On Sun, 24 Jun 2018 at 03:26, Eli Schwartz <[email protected]> wrote: > But Allan's point seems to be that we shouldn't try to stop people from > using variables that makepkg does not utilize, just because they look > vaguely like something makepkg might have used.
The thing is makepkg does use these variables to some extent. Sure pkgname_i686 is ignored. But things like makedepends_i686 do get picked up. So the section would stay, I would instead just loop over a different array of non overridable and non architecture specific fields. I'm all for stricter linting, It doesn't make sense to write pkgname_i686, banning these kind of things would help catch mistakes. > At a bare minimum, if we are going to ban them, the error message and > entire handling logic should be merged into your next patch. Sure I'll squash those two commits if you like. > This error message would have the benefit of being the actual, true > reason we're aborting. (You could check inside the package functions > too, and extend the error message to refer to where they come from.) Better error messages is something I like too. I'll leave it for either after v2 or a different patch set altogether though. I'd like to get the current issues reviewed and out the way first. On Sun, 24 Jun 2018 at 03:26, Eli Schwartz <[email protected]> wrote: > > On 06/23/2018 10:01 PM, Morgan Adamiec wrote: > > Silly mistakes aside. > > > >> These are not variables being overridden... pkgname_i686 is just not a > >> thing as far as makepkg is concerned. > > > > The point of this is specifically disallow things like 'pkgname_i686' > > rather than just ignore them. > > But Allan's point seems to be that we shouldn't try to stop people from > using variables that makepkg does not utilize, just because they look > vaguely like something makepkg might have used. > > At a bare minimum, if we are going to ban them, the error message and > entire handling logic should be merged into your next patch. > > This error message would have the benefit of being the actual, true > reason we're aborting. (You could check inside the package functions > too, and extend the error message to refer to where they come from.) > > > -- > Eli Schwartz > Bug Wrangler and Trusted User >
