Re: [Rpm-maint] [rpm-software-management/rpm] Allow the transaction ID and installation time to be overridden (PR #1803)
@pmatilai commented on this pull request. > @@ -1296,3 +1296,30 @@ rpmtxn rpmtxnEnd(rpmtxn txn) } return NULL; } + +#define FAKE_CLOCK_INITIALIZED (1<<0) +#define FAKE_CLOCK_ACTIVE (1<<1) + +static int _fakeClockState = 0; +static time_t _fakeClock = -1; bogoClock would work for me too :grin: -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1803#discussion_r736291605___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Allow the transaction ID and installation time to be overridden (PR #1803)
@pmatilai commented on this pull request. > @@ -1296,3 +1296,30 @@ rpmtxn rpmtxnEnd(rpmtxn txn) } return NULL; } + +#define FAKE_CLOCK_INITIALIZED (1<<0) +#define FAKE_CLOCK_ACTIVE (1<<1) + +static int _fakeClockState = 0; +static time_t _fakeClock = -1; + +rpm_time_t rpmtsGetTime(time_t step) Dunno, this seems quite nice and simple to me, for the purpose. Remember this is an internal-only API which we can change at will if the need rises, but at the moment I don't see a need for any further API on this. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1803#discussion_r736290223___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Allow the transaction ID and installation time to be overridden (PR #1803)
@pmatilai commented on this pull request. > @@ -1296,3 +1296,30 @@ rpmtxn rpmtxnEnd(rpmtxn txn) } return NULL; } + +#define FAKE_CLOCK_INITIALIZED (1<<0) +#define FAKE_CLOCK_ACTIVE (1<<1) + +static int _fakeClockState = 0; +static time_t _fakeClock = -1; It's not a real clock and the values don't even remotely resemble what you'd get with a real one, so logic says it must be a fake one then :sweat_smile: "synthetic" might be okay as a name but it's on the long side, ditto with "artificial". But considering this is buried deep inside rpm, it hardly matters what we call it. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1803#discussion_r736287274___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Allow the transaction ID and installation time to be overridden (PR #1803)
@cgwalters commented on this pull request. Thank you so much for working on this! > @@ -1296,3 +1296,30 @@ rpmtxn rpmtxnEnd(rpmtxn txn) } return NULL; } + +#define FAKE_CLOCK_INITIALIZED (1<<0) +#define FAKE_CLOCK_ACTIVE (1<<1) + +static int _fakeClockState = 0; +static time_t _fakeClock = -1; (Not an rpm maintainer myself but) The term "fake" has negative connotations, but this is actually going to (IMO) be quite a normal thing to do. Alternative terms: "synthetic", "external", "override" or so? > @@ -1296,3 +1296,30 @@ rpmtxn rpmtxnEnd(rpmtxn txn) } return NULL; } + +#define FAKE_CLOCK_INITIALIZED (1<<0) +#define FAKE_CLOCK_ACTIVE (1<<1) + +static int _fakeClockState = 0; +static time_t _fakeClock = -1; + +rpm_time_t rpmtsGetTime(time_t step) It would seem cleaner to me to have `rpmtsInitTime()` and `rpmtNextTime()` or so, which correspond to `rpmtsGetTime(0)` and `rpmtsGetTime(1)`. > @@ -1296,3 +1296,30 @@ rpmtxn rpmtxnEnd(rpmtxn txn) } return NULL; } + +#define FAKE_CLOCK_INITIALIZED (1<<0) +#define FAKE_CLOCK_ACTIVE (1<<1) + +static int _fakeClockState = 0; It'd be nice to avoid more global mutable state in RPM. Can we actually hook this clock state inside the `ts` structure? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1803#pullrequestreview-788150043___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Allow the transaction ID and installation time to be overridden (PR #1803)
Do use a per-transaction clock, exactly because a consumer may run multiple independent transactions and if they use a shared clock, the results are no longer independently deterministic (populate transactions A and B, run A, then B vs B then A). Also allows both faked and non-faked transactions in the same process. Neither are the most likely of scenarios but static variables are best avoided, especially when we already have a struct we can easily stick it into and using it seems like the right thing to do anyway. (I wonder if we could get by with just one tracking variable for this, eg fakeClock == -1 for uninitialized, 0 for disabled and otherwise active. Two variables seems somehow excessive for the case, even though more correct and as if it somehow actually mattered :sweat_smile: ) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1803#issuecomment-950896601___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Allow the transaction ID and installation time to be overridden (PR #1803)
Alright. I've just force-pushed this with a new spin based on the idea of simply using `SOURCE_DATE_EPOCH` and having a "fake clock" sitting under the relevant parts. I went with a central static clock rather than a per-transaction one in case a consumer is performing multiple transactions. Let me know what you think. Thanks for the feedback! -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1803#issuecomment-948514404___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Allow the transaction ID and installation time to be overridden (PR #1803)
@jeamland pushed 1 commit. d00d9cbf9715f53f903a6cf7e6c33697d4f293b2 Allow an optional "fake clock" for deterministic timestamps -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1803/files/9a2d7a3b915ef77bf100f5413cff474a941605aa..d00d9cbf9715f53f903a6cf7e6c33697d4f293b2 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Allow the transaction ID and installation time to be overridden (PR #1803)
I keep forgetting about `SOURCE_DATE_EPOCH`. Makes me wonder if we need any other mechanism for activating the fake clock at all :thinking: We have a whole pile of related macros which are on/off toggles, so maybe this should suite although I'm not quite sure every single addition really requires its own switch - the naming inconsistency is wonderful... - %source_date_epoch_from_changelog - %use_source_date_epoch_as_buildtime - %clamp_mtime_to_source_date_epoch So feel free to come up with your own scheme or base on the above, %use_source_date_epoch_as_clock (or fakeclock) or whatever. I'm thinking basically: at rpmtsCreate(), see if SOURCE_DATE_EPOCH is set and if so, initialize the fake clock from that value. Add an internal API add something like `time_t rpmtsTime(rpmts ts, int step)` which normally simply returns `time()` but if fake clock is enabled, returns that value instead and increments the time with `step` seconds. Thus, internal callers can decide whether the fake time is passing or not. Not sure the step argument is actually needed for this purpose though. This is just an off-hand idea and there may well be holes in that logic :sweat_smile: so take with a grain of salt. P.S. BDB files for reproducability ... ouch :grimacing: -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1803#issuecomment-948325851___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Allow the transaction ID and installation time to be overridden (PR #1803)
So have something like a `__override_timestamp` macro that, if set, would act a base timestamp that could be used as RPMTAG_INSTALLTID and then you could do something like singular increments of that timestamp as the RPMTAG_INSTALLTIME values? And then `SOURCE_DATE_EPOCH` could be shunted into that macro if it's set? Basically I'm happy with anything that allows me to force some determinism. You should see what I'm having to do to the BerkeleyDB files in our images to make them consistent. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1803#issuecomment-948304501___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Allow the transaction ID and installation time to be overridden (PR #1803)
So have something like a `__override_timestamp` macro that, if set, would act a base timestamp that could be used as RPMTAG_INSTALLTID and then you could do something like singular increments of that timestamp as the RPMTAG_INSTALLTIME values? And then `SOURCE_DATE_EPOCH` could be shunted into that macro if it's set? Basically I'm happy with anything that allows me to force some determinism. You should see what I'm having to do to the BerkeleyDB files in our images to make them consistent. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1803#issuecomment-948304654___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Allow the transaction ID and installation time to be overridden (PR #1803)
@jeamland pushed 3 commits. aad08ca817c3a0bb4a6801559a64735bb0c4d39a Allow overriding of transaction ID and install time. 490d9b6bd1b343c4ec22b3e4a1588177264ea3cc Allow SOURCE_DATE_EPOCH to override the install time and TID. 9a2d7a3b915ef77bf100f5413cff474a941605aa Expose the TID and install time overrides via the Python API. -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1803/files/1da3094be8a261cc26fd922d679cb9e9fe2dd95d..9a2d7a3b915ef77bf100f5413cff474a941605aa ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Allow the transaction ID and installation time to be overridden (PR #1803)
Force-pushed to fix a documentation problem (mis-named parameter in the documentation comment) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1803#issuecomment-948120683___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
[Rpm-maint] [rpm-software-management/rpm] Allow the transaction ID and installation time to be overridden (PR #1803)
The target audience for these changes is people building OS images (or anything else RPM-derived) that desire a reproducible build. The changes are described fairly clearly but the gist is that `RPMTAG_INSTALLTID` and `RPMTAG_INSTALLTIME` contain timestamps and current mechanisms make these impossible to override. These changes add the necessary override APIs to override these values when desired. You can view, comment on, or merge this pull request online at: https://github.com/rpm-software-management/rpm/pull/1803 -- Commit Summary -- * https://github.com/rpm-software-management/rpm/pull/1803/commits/b8d840e6c5e6ac4f1d140fb07916f3d4e8e194d8;>Allow overriding of transaction ID and install time. * https://github.com/rpm-software-management/rpm/pull/1803/commits/bb2d0ede7ba3b217aee8566a5e2c38608c9b727a;>Allow SOURCE_DATE_EPOCH to override the install time and TID. * https://github.com/rpm-software-management/rpm/pull/1803/commits/1da3094be8a261cc26fd922d679cb9e9fe2dd95d;>Expose the TID and install time overrides via the Python API. -- File Changes -- M lib/psm.c (2) M lib/rpmts.c (53) M lib/rpmts.h (28) M lib/rpmts_internal.h (6) M python/rpmts-py.c (30) M rpm.c (10) M tests/rpmi.at (30) M tests/rpmpython.at (92) -- Patch Links -- https://github.com/rpm-software-management/rpm/pull/1803.patch https://github.com/rpm-software-management/rpm/pull/1803.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1803 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
