timelocal_posix/timegm_posix

[Grinnz]

It would be nice to have functions that are guaranteed to be the reverse of the well-known localtime and gmtime functions in core Perl and POSIX. This can be implemented simply with another internal option that prevents any change to the passed in year value so it is always interpreted as years since 1900. I would be happy to provide a PR if nobody gets to this in a week or so.

[autarch]

Yes, I'm all for adding some more functions.

[miyagawa]

I came here to echo this :) _modern is better than the DWIM default but subtracting 1900 should be unnecessary, since I'd expect timelocal is round-trip to localtime functions.

[miyagawa]

can I suggest that we should split that into Time::Local::POSIX instead with timelocal and timegm functions exported instead, rather than adding yet another exported functions.

[Grinnz]

I would rather it be added to Time::Local so that it will be core in future Perls and eventually something we can reliably use.

[Grinnz]

Plus it is a trivial addition in this module, much more difficult as a separate module that cannot depend on the internal state.

[miyagawa]

I didn't necessarily mean a split distro, rather a different package so that you don't need to rewrite the callers and don't need to implement timelocal_posix_nocheck.

[autarch]

I don't like the idea of another package that exports the same names as Time::Local itself. That could easily lead to a situation where a single codebase has instances of timelocal in different modules where the meaning of that sub is very subtly different, so people working on that codebase would always have to look at their imports to figure out which was being used. And of course people would forget to look and assume it was what they expected, leading to weird bugs.

Providing new names from the same package makes it a lot clearer what is being done.

[miyagawa]

Fair. My line of thoughts was that the POSIX version should be the drop-in replacement and existing code should work better and less surprising, because that's how I was using this library. But that's clearly not the case here, especially now that more code is being written in a way it's not round-trip safe to localtime and gmtime.

[autarch]

Hi all, thanks for contributing to this discussion!

I've made a PR with a proposed set of *_posix functions that I would appreciate your feedback on. There are some other changes in that PR like switching to Azure, so if you want to just look at the new subs, you can look at commit 45b1cbf.

[n1vux]

Looks straightforward.

Would an import-option to import timelocal_posix() and timegm_posix() as timelocal() and timegm() make sense to resolve the tension expressed in

While it would be nice to make this the default behavior, that would almost certainly break a lot of code, so you must explicitly import these functions and use them instead of the default C<timelocal()> and C<timegm()>.

[Grinnz]

That would run into the same problem as the earlier drop in replacement suggestion. It should always be clear you are using a different variant of the functions.

[n1vux]

@Grinnz the big problem with drop-in noted above was breaking existing code, so no, not 'the same problem' as already rejected.

The suggested/hypothetical aliasing-import might break existing code in other modules that is used by code using the alias, but it doesn't break everything just by installing it. It is not immediately clear to me that this is a serious danger to new code, which is the only place such an alias should be used. Importing Date::Time after the susceptible legacy module - so it compiles before the alias is created for the importing module - should be sufficient to avoid the problem?

(Obviously ifan alias gets pulled into symbol table of a script or module importing the module importing the aliases, it would only be safe for scripts or darkpan modules. )

One could reasonably claim importing with :posix_aliased or whatever is adequate warning that in this source file, the names are the posix inverses. If that was house style locally, that would indeed be adequate. Arguing otherwise is arguing that no one should ever use the modules Alias and Sub::Alias. Choice is good.

Offhand IDK if there's a suitable way to localize the alias to the source file imported into, but that would be the desirable behavior. If that isn't practical, it's probably not worth the trouble it would cause. If that is practical, making it optionally easier to use the recommended API is a plausible reasonable thing?

[autarch]

@n1vux No, my objection was not that it would break code. My issue is that if there are multiple different behaviors for a timelocal(...) call in a code base, that is incredibly confusing. Every time you saw one you'd have to look at the imports list to see how it was imported in order to determine how it behaved. That's a recipe for confusion.

If different behaviors have different names then the behavior is always apparent from the call itself.

[miyagawa]

the big problem with drop-in noted above was breaking existing code, so no, not 'the same problem' as already rejected.

My drop-in suggestion earlier does not break existing code. It's simply changing use Time::Local to use Time::Local::POSIX in the caller to change the semantics, so yes it pretty much sounds like the same problem to me.

However, like I said, I do not disagree here that it would create a confusion, since it is already a mess for the last 20 years since the DWIM behavior has been introduced in the first place.

[Grinnz]

Arguing otherwise is arguing that no one should ever use the modules Alias and Sub::Alias.

It's a separate problem from changing default behavior, but it's still a problem. I do indeed disagree with the usage of aliasing if it is used to change the behavior of keywords that already have a meaning. In large codebases it is not always trivial to know what has been imported in a particular file. But if you want to shoot yourself in the foot, it's possible without this module encouraging it.

[miyagawa]

Offhand IDK if there's a suitable way to localize the alias to the source file imported into, but that would be the desirable behavior

This can be achieved by putting the following 2 lines in your code where you want the POSIX behavior:

use Time::Local qw(timelocal_posix);
*timelocal = \&timelocal_posix;

and if there're many files that do this you can put it into your own .pm file and export that aliased sub, to avoid repeating yourself. (and if you're sure that all of the instances expect the POSIX behavior).

Personally, I do not expect it to be common that timelocal() is used in hundreds of places even for a large code base (I have a fairly large code base at work and it was used in only one place), but who knows how codes in the wild are maintained.

[n1vux]

Thank you @miyagawa for a sensible discussion. I accept the consensus with your rationale.

Re -

who knows how codes in the wild are maintained

in my experience looking at Darkpan code aka client legacy codebases, one was better off not knowing! :-/ (Some client codebases are much cleaner than others. Very happy with current client's. But some of those others ... ewww!)

Pretty much any horror imaginable is out there somewhere. TIMTOWTDI is like the Force.

[autarch]

I just released 1.29 as a trial version with these new posix variants.

[autarch]

Released in 1.30.