Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NEW PORT] lang/tcc816 - 65816 fork of Tiny C Compiler #189

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

darkain
Copy link

@darkain darkain commented Aug 24, 2023

A fork of the the Tiny C Compiler (TCC) for the WDC 65816 CPU

This CPU is used by the Super Nintendo Entertainment System (SNES)
and the Super Famicom (SFC), as well as other devices from that era.

@darkain darkain changed the title New Port: tcc816 - 65816 fork of Tiny C Compiler New Port: lang/tcc816 - 65816 fork of Tiny C Compiler Aug 24, 2023
@darkain darkain changed the title New Port: lang/tcc816 - 65816 fork of Tiny C Compiler [NEW PORT] lang/tcc816 - 65816 fork of Tiny C Compiler Aug 24, 2023
@@ -0,0 +1,28 @@
PORTNAME= tcc
DISTVERSION= 0.9.25
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please enlighten me. The instructions in the handbook are confusing. It doesn't have clear instructions when using a specific commit hash from a repo. I've been working with the upstream repo to fix FreeBSD specifics there, before a new point release is cut, so I'm using a specific commit hash that is known good rather than an older release that doesn't function on FreeBSD.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example 17 or if upstream plans to use tags/releases --> Example 18

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The version number is the direct version forked from up-up-stream TCC. This particular fork has not established a version numbering scheme yet, its still early and its being figured out how to do things going forward. But this very much is that specific version of TCC, just with a different compile target. Stripping the version information would only serve to confuse users looking at up-up-stream documentation on CLI flags n what not.

Copy link
Contributor

@diizzyy diizzyy Aug 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't lie about version numbering, if there's no versioning we follow what Porters Handbook says and don't make up our own. I'd suggest that you ask upstream what the plan is and if they can at least use tags.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't appreciate being called a liar. I didn't make anything up at all.

This is literally TCC version 0.9.25, with a different target CPU architecture, as I've already described. The only difference between base TCC and this TCC is the target CPU architecture. The entire point of this port is to have the ability to compile code for the 65816 CPU architecture.

https://github.com/alekmaul/tcc/blob/1ba6423fa111f67874649d8d72849dc8a611246e/VERSION

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't say that, what I'm saying that we shouldn't make up our own versions and we follow Porters Handbook on how upstream handles versioning as closely as possible. I'm not going to spend any more time arguing about that, the currently defined version is incorrect and you can simply open an issue at upstream upstream and ask about versioning and how they'll handle it. We're not the only ones who have guidelines about this matter.

While not great 0.9.25.(g)insert-commit-date-here can be acceptable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still missing why g is needed here? Is that because he's created his own repo? Or something else? Since he's controlling his repo, he could create a 0.9.25.{1,2,3,4} tag as he makes changes and is ready to do his release.

Though, to be honest, I thought the version was totally fine and don't understand, at all, what the issue is.

Copy link
Contributor

@diizzyy diizzyy Aug 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

g --> git - Example 17 in Porters Handbook

Controlling what repo? https://github.com/alekmaul/tcc isn't the same user?
Another issue with the current approach is if lets say next update will be 20 commits later, how do we handle that in a sane way? Still report as X and bump PORTREVISION? (That's a very bad idea)


PLIST_FILES= bin/816-tcc

.include <bsd.port.pre.mk>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

¯_(ツ)_/¯ because I used another existing port as a template for this one... unless existing ports are bad..?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You usually only add pre/options if you have if statments otherwise this include is unnecessary and/or incorrect

do-install:
${INSTALL_PROGRAM} ${WRKSRC}/816-tcc ${STAGEDIR}${PREFIX}/bin/

.include <bsd.port.post.mk>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.include <bsd.port.mk>

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be defined unless you also have pre.mk/options.mk defined.

@diizzyy
Copy link
Contributor

diizzyy commented Sep 28, 2023

Hi,

Still interested in getting this committed? If so please address the mentioned issues otherwise please close this PR.

Best regards,
Daniel

@clausecker
Copy link
Contributor

I would like to apologise for the rather harsh treatment you have received at the hand of other project members. Let's hope we can move the port forwards.

Here are the issues that need to be addressed to move the port forwards:

  • when an intermediate state of a project is used, the version of the port should include a suffix of the form .gYYYYMMDD (use PORTVERSION instead of DISTVERSION) to make it clear that this it does not exactly correspond to an upstream version. Even if a project has a version number in the source code, that is often not reliable as projects tend to only change version numbers shortly before they cut a new release. So something like 0.9.25.g20231009 would be an acceptable version number. Or even better, tag a release and use that version.
  • remove .include <bsd.port.pre.mk> and turn .include <bsd.port.post.mk> into just .include <bsd.port.mk>. The two includes are only needed if you need to check for variables defined by the ports framework (e.g. using .if), but that's not the case with your port.

Port looks ok otherwise and we can proceed with further build tests once you address these issues and upload a new revision of the PR.

I also recommend that you submit future new ports on the bug tracker as that's the main way contributions to ports happen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants