Re: [gentoo-dev] [PATCH] versionator.eclass: Add tests for parameter counts

[Kent Fredric]

On Sun, 24 Jul 2016 12:17:56 +0200
Michał Górny  wrote:

> So what's the alternative? Design another eclass where ebuilds will
> fail just the same because people will ignore the more explicit
> requirement just the same as they do ignore the API?

Sometimes you don't need a "new path", you just need to transition from
the old behaviour to the new behaviour at a slower rate with more
visibility.

Option 1: Start off with them being QA Warnings instead of fatal errors
and encourage end users to report them where they see them.

Then after a cycle of warnings, you go through and fatalise them
incrementally in order of "least likely to break the build in dangerous
ways". 

Option 2: Bind the behaviour to an EAPI version that is not yet in use.

Then, when that EAPI gets rolled out to the ebuilds getting upgrades,
the strictures come into effect when the EAPI changes, giving
maintainers fair opportunity to fix the problem before it rolls out to
users.

For me neither of these options say "don't do this thing", they're just
"manage the bleeding better"


pgp4LnC2rcpnQ.pgp
Description: OpenPGP digital signature

Re: [gentoo-dev] [PATCH] versionator.eclass: Add tests for parameter counts

[Alexis Ballier]

On Sun, 24 Jul 2016 10:17:24 +0200
Chí-Thanh Christopher Nguyễn  wrote:
> > Then ebuilds will fail just the same  
> 
> No. Before, ebuilds would maybe display an upgrading message when
> they shouldn't, or vice versa. Now the eclass dies on them.


As a side note: die-ing in anything after merging the package
(>=pkg_postinst) is a seriously bad idea.

Re: [gentoo-dev] [PATCH] versionator.eclass: Add tests for parameter counts

[Chí-Thanh Christopher Nguyễn]

Chí-Thanh Christopher Nguyễn schrieb:

I don't say it is a correct use of versionator.eclass. I just say it has
become (unintentionally) part of the API, and therefore is subject to the
rules about changing APIs in eclasses.


Actually, after reading those rules[1] again, it would be enough to fix all 
ebuilds in the tree and wait 30 days to be in compliance with eclass policy.


So from my side I will no longer object to your proposed change, provided 
these conditions are met.



Best regards,
Chí-Thanh Christopher Nguyễn

[1] https://devmanual.gentoo.org/eclass-writing/

Re: [gentoo-dev] [PATCH] versionator.eclass: Add tests for parameter counts

[Ulrich Mueller]

> On Sun, 24 Jul 2016, Michał Górny wrote:

> There is no such thing as 'unintentional API'. API is what's
> described. If you rely on internals, random undefined behaviors
> or whatever, it's not a part of the API.

Like memcpy(3) direction?

Ulrich


pgpaBBWxp15uE.pgp
Description: PGP signature

Re: [gentoo-dev] [PATCH] versionator.eclass: Add tests for parameter counts

[Jeroen Roovers]

On Sun, 24 Jul 2016 12:17:56 +0200
Michał Górny  wrote:

> But that's a social problem that could easily solved by more
> proactive retirements, and not a good API design point.

You need some trained and payed PR people to correct your writings
before they go to press or you will never be elected president.


 jer

Re: [gentoo-dev] [PATCH] versionator.eclass: Add tests for parameter counts

[Chí-Thanh Christopher Nguyễn]

Michał Górny schrieb:

You are changing the API of versionator.eclass, even if it was unintentional 
API.


There is no such thing as 'unintentional API'. API is what's described.
If you rely on internals, random undefined behaviors or whatever, it's
not a part of the API.


Well there is the API according to the docs, and then there is the API 
according to the implementation...



Then ebuilds will fail just the same


No. Before, ebuilds would maybe display an upgrading message when they
shouldn't, or vice versa. Now the eclass dies on them.


So what's the alternative? Design another eclass where ebuilds will
fail just the same because people will ignore the more explicit
requirement just the same as they do ignore the API?


I don't know what is your problem here. The eclass needs not be designed 
again from the ground up.
All ebuilds which are unaffected by bug 589444 can be switched to the new 
eclass/functions immediately. The others after they are changed no longer 
make the implicit assumption that REPLACING_VERSIONS contains only a single 
version.



The problem is not 'there is a valid case to pass useless parameters
to the function'. The problem is 'I make an invalid assumption about
what will happen based on my limited knowledge which I believe is
more correct than people who wrote package managers'.


I don't say it is a correct use of versionator.eclass. I just say it has 
become (unintentionally) part of the API, and therefore is subject to the 
rules about changing APIs in eclasses. I agree that relying on unintentional 
or undocumented API is bad and needs to be addressed.



Best regards,
Chí-Thanh Christopher Nguyễn

Re: [gentoo-dev] [PATCH] versionator.eclass: Add tests for parameter counts

[Michał Górny]

On Sun, 24 Jul 2016 10:17:24 +0200
Chí-Thanh Christopher Nguyễn  wrote:

> Michał Górny schrieb:
> > So, to summarize we shouldn't fix existing code because people did
> > assume accepting invalid parameters was fine.  
> 
> You are changing the API of versionator.eclass, even if it was unintentional 
> API.

There is no such thing as 'unintentional API'. API is what's described.
If you rely on internals, random undefined behaviors or whatever, it's
not a part of the API.

Yes, I know that most of Gentoo developers are bright enough to assume
'if it is not explicitly forbidden in words I can't find a way to bend
to pretend it's allowed anyway, it's allowed even if it doesn't make
any sense'. But that's a social problem that could easily solved by more
proactive retirements, and not a good API design point.

> > Then ebuilds will fail just the same  
> 
> No. Before, ebuilds would maybe display an upgrading message when they 
> shouldn't, or vice versa. Now the eclass dies on them.

So what's the alternative? Design another eclass where ebuilds will
fail just the same because people will ignore the more explicit
requirement just the same as they do ignore the API?

The problem is not 'there is a valid case to pass useless parameters
to the function'. The problem is 'I make an invalid assumption about
what will happen based on my limited knowledge which I believe is
more correct than people who wrote package managers'.

-- 
Best regards,
Michał Górny



pgpQTqIRQWLXO.pgp
Description: OpenPGP digital signature

Re: [gentoo-dev] [PATCH] versionator.eclass: Add tests for parameter counts

[Jason Zaman]

On Sun, Jul 24, 2016 at 10:17:24AM +0200, Chí-Thanh Christopher Nguyễn wrote:
> Michał Górny schrieb:
> > So, to summarize we shouldn't fix existing code because people did
> > assume accepting invalid parameters was fine.
> 
> You are changing the API of versionator.eclass, even if it was unintentional 
> API.

# @FUNCTION: version_is_at_least
# @USAGE:  [have]
# @DESCRIPTION:
# Is $2 (defaults to $PVR) at least version $1? Intended for use in eclasses
# only. May not be reliable, be sure to do very careful testing before actually
# using this.

The API seems pretty clear to me ...

-- Jason

Re: [gentoo-dev] [PATCH] versionator.eclass: Add tests for parameter counts

[Chí-Thanh Christopher Nguyễn]

Ciaran McCreesh schrieb:

On Sun, 24 Jul 2016 10:17:24 +0200
Chí-Thanh Christopher Nguyễn  wrote:

Then ebuilds will fail just the same


No. Before, ebuilds would maybe display an upgrading message when
they shouldn't, or vice versa. Now the eclass dies on them.


This attitude that invisible failures (which don't get fixed, and which
lead to occasional weirdness) are better than visible failures (which
must be fixed) is an odd one... Postel has a lot to answer for.


I would agree with you when it comes to upstreams' -Werror but I do realize 
that I am in the minority there.


My point here is that this change will affect the behaviour of ebuilds using 
this eclass.



Best regards,
Chí-Thanh Christopher Nguyễn

Re: [gentoo-dev] [PATCH] versionator.eclass: Add tests for parameter counts

[Ciaran McCreesh]

On Sun, 24 Jul 2016 10:17:24 +0200
Chí-Thanh Christopher Nguyễn  wrote:
> > Then ebuilds will fail just the same  
> 
> No. Before, ebuilds would maybe display an upgrading message when
> they shouldn't, or vice versa. Now the eclass dies on them.

This attitude that invisible failures (which don't get fixed, and which
lead to occasional weirdness) are better than visible failures (which
must be fixed) is an odd one... Postel has a lot to answer for.

-- 
Ciaran McCreesh

Re: [gentoo-dev] [PATCH] versionator.eclass: Add tests for parameter counts

[Chí-Thanh Christopher Nguyễn]

Michał Górny schrieb:

So, to summarize we shouldn't fix existing code because people did
assume accepting invalid parameters was fine.


You are changing the API of versionator.eclass, even if it was unintentional 
API.


Then ebuilds will fail just the same


No. Before, ebuilds would maybe display an upgrading message when they 
shouldn't, or vice versa. Now the eclass dies on them.



Best regards,
Chí-Thanh Christopher Nguyễn

Re: [gentoo-dev] [PATCH] versionator.eclass: Add tests for parameter counts

[Michał Górny]

On Sun, 24 Jul 2016 00:17:21 +0200
Chí-Thanh Christopher Nguyễn  wrote:

> Michał Górny schrieb:
> > Ensure that proper number of parameters is passed to each versionator
> > function; die otherwise. This prevents the functions from proceeding
> > with undefined behavior when mis-called.  
> 
> You are making what versionator.eclass accepts stricter. That it used to work 
> when passed multiple versions may be unintentional, but I think you need to 
> introduce a new eclass or new function names in this case.

So, to summarize we shouldn't fix existing code because people did
assume accepting invalid parameters was fine.

What we should do instead is create an almost identical copy of it, ask
people to switch to new code with parameter checks. But... why?

If people already pass valid parameters, then new and old code would be
functionally identical. If they do not, then they can fix parameters
and stay with the old code. But they shouldn't have been doing that in
the first place...

So in the end, we create another copy of the functions that's used
inconsistently with the old copy, and only when people feel like
switching. Then ebuilds will fail just the same because people wouldn't
care to *ensure* they pass valid parameters while switching, as long as
they won't trigger it on their limited test case...

-- 
Best regards,
Michał Górny



pgpUMSncJP8S8.pgp
Description: OpenPGP digital signature

Re: [gentoo-dev] [PATCH] versionator.eclass: Add tests for parameter counts

[Chí-Thanh Christopher Nguyễn]

Michał Górny schrieb:

Ensure that proper number of parameters is passed to each versionator
function; die otherwise. This prevents the functions from proceeding
with undefined behavior when mis-called.


You are making what versionator.eclass accepts stricter. That it used to work 
when passed multiple versions may be unintentional, but I think you need to 
introduce a new eclass or new function names in this case.



Best regards,
Chí-Thanh Christopher Nguyễn