On Thu, 18 Jan 2024, Thomas Frohwein wrote:

> On Thu, Jan 18, 2024 at 01:25:35PM -0500, Daniel Dickman wrote:
> > The latest upstream commit drops legacy Python 2 support code.
> 
> Interesting that they are refactoring this much while the project goal
> seems to be the move to a Godot4-based engine [1].

To be honest I'm really happy that they continue to make small 
improvements while the work on the godot engine continues.

> 
> > Following this update we can drop the RDEP on py-future. However testing 
> > this change revealed that fifengine was missing an RDEP on py-future. The 
> > fix for fifengine is already in the tree, but I updated the fifengine RDEP 
> > here to 0.4.2p4 to make sure py-future continues to stay around at 
> > runtime.
> 
> Not sure if specifying the min version of fifengine is necessary if
> -current only has the latest version anyway. Installation should anyway
> be done with pkg_add -U if packages aren't up-to-date...

The only downside that was raised to me in the past is that when EPOCH is 
bumped, one has to remember to review if version constraints in other 
ports need to be bumped which isn't ideal. Don't disagree there, but I 
plan to keep the RDEP bump as it can help prevent a runtime problem. If it 
were a BDEP, I'd remove the version constraint though.

> 
> > The TDEP on py-nose was removed upstream in commit 3a808be which switched 
> > to pytest. (But tests are still broken as they do not support pytest 4+ 
> > yet.)
> > 
> > Also switch to using MODPY_SETUPTOOLS=Yes which seems to work (possibly 
> > after upstream commit 50397fa.
> > 
> > ok?
> 
> Tested and builds and works, but I'm getting a slightly different PLIST.

What is the PLIST difference you're seeing?

> I also understand the difference between REVISION and the version in
> PKGNAME such that REVISION is for when the port changes with same
> upstream version generally, and PKGNAME changes with change in upstream
> version (cf. packages-specs(7)). If this is accurate, using a more
> recent GH_COMMIT would mean changing the PKGNAME. I would propose
> updating it to 2019.1pl0 and dropping REVISION...

There are a few different patterns for using a git commit when there isn't 
a release and this is the one I've personally found to be least 
problematic in case upstream does a point release.

In the past I've used "version.date" and added the date of the git commit 
but that caused problems if upstream ended up doing some kind of point 
release. eg. say we have 13.1.20240110 and then upstream releases 
13.1.1...

Also we should consider impact on portroach and 3rd party sites like 
repology if we use a different approach than bumping REVISION when 
updating to a newer commit. It's ideal if those sites don't get confused 
as well.

I plan to keep as is but happy to revisit if there's different guidance 
for the ports tree. Don't think I want to make this game the odd one out 
though.

> 
> Per bsd.port.mk(5), NO_TEST shouldn't be used for ports with existing,
> but failing tests. Therefore, I suggest removing the NO_TEST line.

This isn't a case of having some failing tests, rather it's a case of 
the entire test framework not being runnable under our pytest which is 
too new. And given we don't support multiple pytest versions there isn't 
much we can do here without upstream solving for the pytest 4.x 
deprecations.

By the way, nothing to do with unknown horizons, but one idea for the 
ports framework would be to support NO_TEST=comment and then print comment 
when trying to run the tests.

Right now if you remove NO_TEST from this port, someone would likely waste 
their time trying to understand what's going on (unless that person is 
very well versed with the pytest evolution).

I also think the guidance in bsd.port.mk could be improved. For example I 
really think tests should be reviewed with every port update. The current 
guidance is a bit too simplistic for the real world where tests sometimes 
are or aren't published on pypi, for example, and sometimes need pytest 
changes or new RDEPs, etc, etc.

p.s. for fun I scanned the tree for ports that use NO_TEST set to 
something other than Yes, and found a few funny ones. For example:

$ grep NO_TEST /usr/ports/net/dsocks/Makefile
NO_TEST=                Ues

> 
> My own adjusted diff is below, but it builds and runs so with or
> without the changes I'm suggesting, ok thfr@.
> 

Again, thanks for the review. I guess I'd just be curious to know what the 
PLIST difference is.

Reply via email to