Skip to content

Comments

Convert timestamp to UTC#1397

Merged
dgdavid merged 1 commit intoyast:masterfrom
bmwiedemann:master
Jan 2, 2025
Merged

Convert timestamp to UTC#1397
dgdavid merged 1 commit intoyast:masterfrom
bmwiedemann:master

Conversation

@bmwiedemann
Copy link
Contributor

Convert timestamp to UTC
to make parsing unambiguous.

Without this change, osc build mis-parsed this as another date
which breaks reproducible builds

to make parsing unambiguous.

Without this change, osc build mis-parsed this as another date
which breaks reproducible builds
Copy link
Member

@dgdavid dgdavid left a comment

Choose a reason for hiding this comment

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

Hi!

Thanks a lot for the requested change.

I'm not against of merging it, but I'd like to know a bit more about the rationale and/or problem before going ahead 😅

Probably due to my knowledge of OBS internals, but according to the PR description it looks a bit weird to me that only the changed WEST entry is the problematic one since there are a bunch of CEST entries in the changes file.

Moreover, there are (not too much) WEST and CEST entries across the YaST modules. See https://github.com/search?q=org%3Ayast+%22+WEST+%22+path%3A*.changes&type=code and https://github.com/search?q=org%3Ayast+%22+CEST+%22+path%3A*.changes&type=code

I've tried to find some information about the UTC requirement at OBS documentation without success. So, I wonder if it is about a bug in the OBS side or just I'm missing something.

May I ask you to provide a bit more information for a better understanding of the problem?

Thanks a lot!

@jreidinger
Copy link
Member

Well, we usually create such entries with osc vc so I kind of hope that osc build should parse it correctly. Maybe it should be fixed on osc build side?
But of course this PR is ok as it is and there is not problem to merge it. Just do not expect we will actively change it everywhere.

@dgdavid dgdavid merged commit 014a74c into yast:master Jan 2, 2025
@github-actions
Copy link

github-actions bot commented Jan 2, 2025

✅ Autosubmission job #12581303411 successfully finished

@bmwiedemann
Copy link
Contributor Author

I'm not sure how exactly it happened. What I observed was that on one machine (with LC_CTYPE=en_DK.utf8; LANG=en_US.utf8), osc build produced .src.rpms with .spec files that had a date that was one day earlier than what the .changes file said.

And I observed it for timestamps that had an hour >12 , including one with 18:00 PM UTC

Some other yast packages in ring1 were also affected:
autoyast2, libyui, yast2, yast2-caasp, yast2-network, yast2-nfsserver, yast2-registration

@bmwiedemann
Copy link
Contributor Author

Here is some more detail on how it failed:
/usr/lib/build/changelog2spec uses /usr/lib/perl5/vendor_perl/5.40.0/Time/Zone.pm which knows about CEST, but not WEST or IST.

perl -e 'use Time::Zone; print tz_offset("CEST"),"\n"'
7200
perl -e 'use Time::Zone; print tz_offset("WEST"),"\n"'

I made a simple toolchain patch at openSUSE/obs-build#1047 so we don't need to patch any more .changes entries.

@dgdavid
Copy link
Member

dgdavid commented Jan 3, 2025

@bmwiedemann

Thanks a ton! For both, the input about the problem and the patch to fix it! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants