[MacPorts] #47896: submission: cpuid

[MacPorts]

#47896: submission: cpuid
--------------------------+--------------------------------
  Reporter:  rjvbertin@…  |      Owner:  macports-tickets@…
      Type:  submission   |     Status:  new
  Priority:  Normal       |  Milestone:
 Component:  ports        |    Version:
Resolution:               |   Keywords:
      Port:  cpuid        |
--------------------------+--------------------------------

Comment (by ionic@…):

 Replying to [comment:4 rjvbertin@…]:
 > Replying to [comment:3 ionic@…]:
 >
 > > the input is meant as a way of learning how "stuff is done", not to
 annoy anyone.
 >
 > It may be just me, but I often find the tone of that input often
 bordering insult ... and to be really educational one would not let the
 pupil ask "how it's done" after being told "that's not how it's done" ;)

 No, it's not meant as an insult. Yes, it can come off as harsh, but it's
 not meant to be that either. I've had lengthy discussions with other
 professionals when reviewing their patches and they assured me that my
 points of critique were spot-on and it's better to think about something
 twice then mess it up first and fix it later. I stopped constantly
 apologizing after pointing out it's just dry, not harsh or insulting.


 > > If you had googled the first sentence ("Permission to use, copy
 [...]") you would have directly landed here:
 https://en.wikipedia.org/wiki/ISC_license
 >
 > The thought did occur to me, but that language is so common that I was
 sure to get hits from lots of comparable but not identical licenses.

 Yeah, I thought that too, but interestingly it lead right to the correct
 license - and I tried anyway, even with having that thought in mind. Even
 *if* multiple licenses were returned, I would have gone through them to
 find the correct one.


 > > There's at least one `\n` that shouldn't be there. We normally don't
 expect empty lines in `long_description` or `description`.
 >
 > That's a \n, not a \n\n, so causes only a newline, not an empty line.

 Yes, you're right, sorry.


 > I like to respect the formatting the author applied, as far as possible.

 See Larry's answer on that, we don't format these things.


 > I'd have noticed if it had overridden any of my settings, or if
 something hadn't worked O:-)

 Also here.


 > I don't think I have trace mode in my version of base, but I shouldn't
 have to.

 Base supports trace mode as of version 2.3.0. If you don't have that, you
 should really, really, really upgrade...


 > No attempt is made to link to libraries from MacPorts.

 It's not just about libraries, but also headers and other stuff, which you
 will *not* see by looking at the compile output.


 > You think I didn't test the application before submitting the portfile?

 No, but...

 > Of course it works.

 I don't think you tested everything. Including old OS X versions, new
 ones, passing combinations of long, short options with and without
 parameters and invalid options.

 Personally, I probably wouldn't test older versions either, because I
 don't have a quick way of doing so, but definitely the other stuff if I'm
 unsure about how the program would behave with getopt as shipped by Apple.


 > The code doesn't check for NO_GNU_GETOPT; the shipped version *is* BSD
 getopt which can apparently be used without build-time changes to the
 code. I don't think there's any point in forcing a link to libgnugetopt.

 Okay, they are shipping NetBSD's getopt. That's probably compatible
 enough.

 [hr]

 Regarding your new Portfile: looks better.

 Please change the ID line to just
 {{{
 # $Id$
 }}}

 as already suggested (c.f.,
 [https://guide.macports.org/chunked/development.creating-portfile.html
 Section 4.2 of the MacPorts Development Guide])

 I'd move `github.setup` up and remove the `name` and `version` variables.
 They are automatically set by `github.setup`.

 I don't really like putting the replacement stuff into `configure`. It's
 really patching, not running a configure script or program. But as it's
 more or less indeed "configuring" the package, I guess we can leave it as-
 is.

 However, there are two problems with that: because you used `use_configure
 no`, there won't be an implicit `universal` variant. Please add one via
 `variant universal {}` after `use_configure no`.

 Secondly, you're way overcomplicating things.

 Drop the `variant_isset universal` check.

 `reinplace` `-Os` with `${configure.cppflags} ${configure.cflags}
 [get_canonical_archflags cc]`
 and
 `LDFLAGS := -lm` with `LDFLAGS := -lm ${configure.ldflags}
 [get_canonical_archflags ld]`

 Make sure to not drop `-lm` again!

 Don't `reinplace` `CC`.

 Rather, append `CC=${configure.cc} LD=${configure.cc}` to `build.args`
 (outside of `configure` of course.)

-- 
Ticket URL: <https://trac.macports.org/ticket/47896#comment:6>
MacPorts <https://www.macports.org/>
Ports system for OS X