Opened 8 years ago

Closed 8 years ago

#53750 closed update (wontfix)

patchfile for ftidy

Reported by: gaming-hacker (G Alexander) Owned by:
Priority: Normal Milestone:
Component: ports Version: 2.4.1
Keywords: haspatch Cc: ryandesign (Ryan Carsten Schmidt)
Port: ftidy

Description

all, i've updated this code, stripped out some legacy deadwood.

here is a patch.

Attachments (1)

Portfile-rrdtool.diff (2.6 KB) - added by gaming-hacker (G Alexander) 8 years ago.
patchfile

Download all attachments as: .zip

Change History (4)

Changed 8 years ago by gaming-hacker (G Alexander)

Attachment: Portfile-rrdtool.diff added

patchfile

comment:1 Changed 8 years ago by ryandesign (Ryan Carsten Schmidt)

Cc: ryandesign added
Keywords: haspatch added; patch update removed

Thanks, but there are many problems with your proposed changes.

You've updated the port's version, but you haven't removed the revision line.

md5 and sha1 checksums are no longer considered secure. You should replace the md5 and sha1 checksums with rmd160 and sha256 checksums (unless upstream provides md5 or sha1 checksums, in which case use those in addition to rmd160 and sha256 checksums).

The biggest problem is that you've changed the version to 7.3, but the distfile is still called tidy72sc.zip. That can't possibly be right, can it? You've changed the homepage, but I assume you've only done that because the old homepage seems to have been deleted. The new homepage you've listed isn't the official homepage. We could use an archived copy of the original homepage.

In fact, I don't believe version 7.3 exists. The tidy.zip file on the new homepage you linked to contains the same 1999 source code as the 7.2 file we were using, but is 4 times larger because it includes precompiled executables which we don't want.

You've added build arguments, including optimization flags and arch flags. Optimization flags go in configure.optflags, but because the port doesn't use a configure phase, you'll then have to use that variable elsewhere. Arch flags are available in configure.cc_archflags. You shouldn't assume the arch is x86_64 nor that SSE is available. (What if the user is on a PowerPC processor?) If the software doesn't support all 4 Mac archs, then supported_archs should be used to indicate which archs are supported.

You've removed the presumably functional gcc43, gcc44, gcc45, gcc46, gcc47 variants and added a nonfunctional gcc4 variant which uses the nonexistent "macports-gcc-4" compiler. GCC changed its version numbering scheme with version 5. Previously, the major version included the first two digits of the version number (4.8, 4.9, etc.) As of version 5, only the first digit is the major version number.

You haven't adjusted the code that selects which of the gcc variants to use by default, so you're still using the now-nonexistent gcc47 variant by default. You should use the latest stable version of gcc (currently gcc6) by default. You may even want to use the compilers 1.0 portgroup instead of creating the variants by hand, but I cannot advise on how to use that portgroup.

comment:3 Changed 8 years ago by ryandesign (Ryan Carsten Schmidt)

Resolution: wontfix
Status: newclosed

I think we've addressed all the parts of this patch that we're going to address, and won't do the rest.

Note: See TracTickets for help on using tickets.