Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
@ffesti pushed 2 commits. c0bb2aaf51f78ac30e86e65f772c17d4c8dde8ae Add rpmlogOnce() and rpmlogReset() 02750bb69f9ad931fe26bef15a4b2615cdfcc27b Use rpmlogOnce() in handleHdrVS -- View it on GitHub: https://github.com/rpm-software-management/rpm/pull/3417/files/bf6baaa3ef7d62959064583d5ead2d64a18fda0c..02750bb69f9ad931fe26bef15a4b2615cdfcc27b You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
@pmatilai commented on this pull request.
> @@ -412,3 +415,43 @@ void rpmlog (int code, const char *fmt, ...)
exit:
errno = saved_errno;
}
+
+int rpmlogOnce (uint64_t domain, const char * key, int code, const char *fmt,
...)
+{
+int saved_errno = errno;
+rpmlogCtx ctx = rpmlogCtxAcquire();
+int newkey = 0;
+
+if (ctx) {
+ wrlock lock(ctx->mutex);
+ newkey = !ctx->seen[domain][{code, key}]++;
Maybe what it really needs is a high level comment stating what it does, and
leave the understanding as an exercise for the reader as per their C++
experience :sweat_smile:
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3417#discussion_r1824170519
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
OK; this should address all issues except for the Python based test case which I will add next. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/3417#issuecomment-2449444799 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] `%{gsub:...}` throws error (Issue #3410)
That's caused by %len being mapped to lua's string.len() function, and lua
does not complain about the extra arg:
```
$ lua
> print(string.len("foo", "bar"))
3
>
```
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/3410#issuecomment-2449450410
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
@dmnks commented on this pull request.
> @@ -412,3 +415,43 @@ void rpmlog (int code, const char *fmt, ...)
exit:
errno = saved_errno;
}
+
+int rpmlogOnce (uint64_t domain, const char * key, int code, const char *fmt,
...)
+{
+int saved_errno = errno;
+rpmlogCtx ctx = rpmlogCtxAcquire();
+int newkey = 0;
+
+if (ctx) {
> It is not. It is there to limit the scope of the lock. If you want to use
> rpmlock below to print the numbers for the repeated messages we can't hold a
> lock there.
Oh right, that didn't occur to me, thanks for clarifying.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3417#discussion_r1824064306
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
@ffesti commented on this pull request.
> @@ -412,3 +415,43 @@ void rpmlog (int code, const char *fmt, ...)
exit:
errno = saved_errno;
}
+
+int rpmlogOnce (uint64_t domain, const char * key, int code, const char *fmt,
...)
+{
+int saved_errno = errno;
+rpmlogCtx ctx = rpmlogCtxAcquire();
+int newkey = 0;
+
+if (ctx) {
+ wrlock lock(ctx->mutex);
+ newkey = !ctx->seen[domain][{code, key}]++;
Yeah this needs a comment. Is `/* This gets initialized automatically on first
access */` clear enough?
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3417#discussion_r1824114894
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
@ffesti pushed 2 commits. f83a969b903a6cef614a2daa1e59650e1eeb0742 Add rpmlogOnce() and rpmlogReset() 7b3377bb0c2bff99a06c5dd4d3aa7ef560549ccf Use rpmlogOnce() in handleHdrVS -- View it on GitHub: https://github.com/rpm-software-management/rpm/pull/3417/files/02750bb69f9ad931fe26bef15a4b2615cdfcc27b..7b3377bb0c2bff99a06c5dd4d3aa7ef560549ccf You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
recs is not really suited for this. It is a vector so lookups are not cheap. It only stores WARNING and above. We might want to link to the recs entry but even that does not really solve anything if the lower prio entries are missing. The additional string is there for cases where the message contains differing info. E.g. the package name or location of the error. Imagine suppressing a missing macro message. This will give a file and line number. And you do want that so you can look at the place where things go wrong in case it is just a typo. You still want to suppress the follow up messages if the macro is actually missing. So having a key allows to put them all under the same category. I don't think we will have logging objects for every possible occasion but only for majors things like builds or transactions. So I expect the domain to stay relevant even if we no longer have global logging. But that might just me being unimaginative. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/3417#issuecomment-2449257525 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
OK, test added. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/3417#issuecomment-2450139574 You are receiving this because you are subscribed to this thread. Message ID:___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmLogOnce to filter log message (Issue #3395)
> The lifetime could be handled by throwing in a context or id to the mix, this could be an arbitrary 64bit int so you can pass in a pointer (ts, spec, whatever), have the keys hang off that Realized today with some amusement that this was very much a C hacker proposal. We, we could just use a std::string for the domain without it being any harder :smile: Not saying that you should change the current PR (we can always do it later) but certainly wont oppose if you do. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/3395#issuecomment-2450090929 You are receiving this because you are subscribed to this thread. Message ID:___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Implement a native C++ macro API + use it to replace manual macro locking (PR #3408)
This looks do to me now. May be @dmnks can have a look and second opinion before merging. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/3408#issuecomment-2450223428 You are receiving this because you are subscribed to this thread. Message ID:___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
[Rpm-maint] [rpm-software-management/rpm] RPM db does not upgrades when upgrading from `4.11.3` to `4.18.2` (Issue #3420)
**Describe the bug** Hi team, I'm working on a Centos 7 Docker container which brings RPM version `4.11.3`. I'm able to cover the required deps and install the `4.18.2` version, but I can not migrate the native rpm db to the installed one and the newer version never knows about the already installed packages. **To Reproduce** * Steps to reproduce the behavior: On a Centos 7 host, install the RPM `4.18.2` with the following command (_having the required deps covered_): ```command git clone https://github.com/rpm-software-management/rpm.git --branch rpm-4.18.2-release --single-branch && \ cd rpm && ./autogen.sh && make -j$(nproc) && make install && cd / && rm -rf rpm* ``` Then I try to migrate the installed packages information with the native RPM version as follows: ```command mkdir -p /usr/local/var/lib/rpm && cp /var/lib/rpm/Packages /usr/local/var/lib/rpm/Packages && \ /usr/local/bin/rpm --rebuilddb ``` But when querying the installed packages list, it seems to not be updated: ```command [root@a0607f3a3f16 /]# /usr/local/bin/rpm --version RPM version 4.18.2 [root@a0607f3a3f16 /]# /usr/local/bin/rpm -qa ``` **Expected behavior** This procedure has been working for me if the new installed RPM version was the `4.15.1`. With this version the packages list is upgraded and the new version knows the previously installed packages. Is it expected to not work with the newer mentioned version? -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/3420 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
@ffesti pushed 1 commit. bf6baaa3ef7d62959064583d5ead2d64a18fda0c Add rpmlogOnce() and rpmlogReset() -- View it on GitHub: https://github.com/rpm-software-management/rpm/pull/3417/files/7eb7b473a04ccad9457bbd0de1bc6c7f52d11b93..bf6baaa3ef7d62959064583d5ead2d64a18fda0c You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
The existing tests don't cover #3336 which this claims to be fixing. Rpm wont do multiple transactions in a process, but you should be able to test it with the python bindings. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/3417#issuecomment-2449318456 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
> I don't think we will have logging objects for every possible occasion but > only for majors things like builds or transactions. So I expect the domain to > stay relevant even if we no longer have global logging. Yeah, logging is kind of global in nature, I don't see that fundamentally changing. The deal with *allowing* non-global context is to allow something like dnf to have a context *they* control, not so much with having multiple different contexts active within rpm at the same time. So I don't see this logonce and domain overlapping with that stuff much, if at all really. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/3417#issuecomment-2449308014 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
@pmatilai commented on this pull request.
> @@ -412,3 +415,43 @@ void rpmlog (int code, const char *fmt, ...)
exit:
errno = saved_errno;
}
+
+int rpmlogOnce (uint64_t domain, const char * key, int code, const char *fmt,
...)
+{
+int saved_errno = errno;
+rpmlogCtx ctx = rpmlogCtxAcquire();
+int newkey = 0;
+
+if (ctx) {
Don't add a bogus check just to create a scope, if you need a scope then just
use a plain { } block.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3417#discussion_r1824045133
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
Also, this is two entirely different things in a single commit. Split them up. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/3417#issuecomment-2449321967 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
> recs is not really suited for this. It is a vector so lookups are not cheap. > It only stores WARNING and above. We might want to link to the recs entry but > even that does not really solve anything if the lower prio entries are > missing. Sure, the logic for populating `recs` would have to be adjusted, my point is that message de-duplication can also be thought of as filtering, and we already do by-mask filtering in `rpmlogPrintByMask()`. What I'm getting at is that we could just record all the messages and only decide how to collapse (de-duplicate) them into a single meaningful message after the fact, perhaps even using the information in those messages somehow. Yes, we need to do that on-the-fly as well, of course, but that would just be a special case of that. That said, I haven't thought this through too much, it's just my gut feeling speaking :smile: Having this specialized `seen` map also works and we can always refactor it in the future, it's not a big deal. > The additional string is there for cases where the message contains differing > info. E.g. the package name or location of the error. Imagine suppressing a > missing macro message. This will give a file and line number. And you do want > that so you can look at the place where things go wrong in case it is just a > typo. You still want to suppress the follow up messages if the macro is > actually missing. So having a key allows to put them all under the same > category. Indeed. I had this in the back of my mind too, just needed to see it written down for it to click. Thanks! > I don't think we will have logging objects for every possible occasion but > only for majors things like builds or transactions. So I expect the domain to > stay relevant even if we no longer have global logging. But that might just > me being unimaginative. Right, it's just that "context" coincides with "domain" in a way, but we already use the term "context" for a different purpose in the logging module, so yep. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/3417#issuecomment-2449336976 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
> Yeah, logging is kind of global in nature, I don't see that fundamentally > changing. The deal with _allowing_ non-global context is to allow something > like dnf to have a context _they_ control, not so much with having multiple > different contexts active within rpm at the same time. So I don't see this > logonce and domain overlapping with that stuff much, if at all really. It just seems to me like this domain tracking (based on an integer key) tries to emulate proper OOP encapsulation. IOW, if an object needs a log "context", it might as well get one, represented as an actual object. The users wanting to emit messages for that context would then pass the object to the logging functions, much like we now pass the domain integer. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/3417#issuecomment-2449347101 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
@pmatilai commented on this pull request.
> @@ -412,3 +415,43 @@ void rpmlog (int code, const char *fmt, ...)
exit:
errno = saved_errno;
}
+
+int rpmlogOnce (uint64_t domain, const char * key, int code, const char *fmt,
...)
+{
+int saved_errno = errno;
+rpmlogCtx ctx = rpmlogCtxAcquire();
+int newkey = 0;
+
+if (ctx) {
But a cleaner version is actually to wrap the seen-check and the lock into a
small seenLog() style helper function.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3417#discussion_r1824047197
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
@pmatilai commented on this pull request.
> @@ -412,3 +415,43 @@ void rpmlog (int code, const char *fmt, ...)
exit:
errno = saved_errno;
}
+
+int rpmlogOnce (uint64_t domain, const char * key, int code, const char *fmt,
...)
+{
+int saved_errno = errno;
+rpmlogCtx ctx = rpmlogCtxAcquire();
+int newkey = 0;
+
+if (ctx) {
+ wrlock lock(ctx->mutex);
+ newkey = !ctx->seen[domain][{code, key}]++;
Yeah there's a lot of magic in that line, perhaps a bit too much. This isn't a
LoC competition, understandability is 100x more important. If @ffesti himself
needed to think for a while to convince himself why this works, that's a danger
sign already, maybe this could be opened up a bit, or at least add a comment.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3417#discussion_r1824094315
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
@pmatilai commented on this pull request.
> @@ -412,3 +415,43 @@ void rpmlog (int code, const char *fmt, ...)
exit:
errno = saved_errno;
}
+
+int rpmlogOnce (uint64_t domain, const char * key, int code, const char *fmt,
...)
+{
+int saved_errno = errno;
+rpmlogCtx ctx = rpmlogCtxAcquire();
+int newkey = 0;
+
+if (ctx) {
+ wrlock lock(ctx->mutex);
+ newkey = !ctx->seen[domain][{code, key}]++;
Mind you, I don't find this unreadable, just needing a bit of thought too. I
guess the thing that raises *my* alarms on first sight is the unchecked []
access which is fine with a map but undefined behavior in some others.
Considering we're all relative C++ beginners here, some such remark would not
be out of line.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3417#discussion_r1824104313
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
@ffesti commented on this pull request.
> @@ -412,3 +415,43 @@ void rpmlog (int code, const char *fmt, ...)
exit:
errno = saved_errno;
}
+
+int rpmlogOnce (uint64_t domain, const char * key, int code, const char *fmt,
...)
+{
+int saved_errno = errno;
+rpmlogCtx ctx = rpmlogCtxAcquire();
+int newkey = 0;
+
+if (ctx) {
It is not. It is there to limit the scope of the lock. If you want to use
rpmlock below to print the numbers for the repeated messages we can't hold a
lock there.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3417#discussion_r1824017885
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Implement a native C++ macro API + use it to replace manual macro locking (PR #3408)
@pmatilai pushed 2 commits. b3731ba589f72035085d41c14f2016d901954d37 Replace macro locking with an internal RAII-style C++ API 2a19bcd79cde0fdbbf769505c5b357c9ff10dc78 Convert a couple of rpmExpand() usages to the C++ versions -- View it on GitHub: https://github.com/rpm-software-management/rpm/pull/3408/files/dbb6a49e401b4696f9570d01e3aea7272c0ea726..2a19bcd79cde0fdbbf769505c5b357c9ff10dc78 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Implement a native C++ macro API + use it to replace manual macro locking (PR #3408)
Added a bunch of notes on the overall design + notes on API differences where they exist. Also added native expand_numeric() variants which I had somehow missed in the first round. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/3408#issuecomment-2449273205 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
@pmatilai commented on this pull request.
> + }
+ va_end(ap);
+}
+errno = saved_errno;
+return newkey;
+}
+
+void rpmlogReset(uint64_t domain, int mode=0)
+{
+rpmlogCtx ctx = rpmlogCtxAcquire();
+std::map, int> domain_data = {};
+
+if (ctx) {
+ wrlock lock(ctx->mutex);
+ if (mode)
+ domain_data = ctx->seen[domain];
Don't add speculative dead code, this only looks baffling. Add handling for
other modes when you add them.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3417#pullrequestreview-2407327566
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
@dmnks commented on this pull request.
> @@ -412,3 +415,43 @@ void rpmlog (int code, const char *fmt, ...)
exit:
errno = saved_errno;
}
+
+int rpmlogOnce (uint64_t domain, const char * key, int code, const char *fmt,
...)
+{
+int saved_errno = errno;
+rpmlogCtx ctx = rpmlogCtxAcquire();
+int newkey = 0;
+
+if (ctx) {
+ wrlock lock(ctx->mutex);
+ newkey = !ctx->seen[domain][{code, key}]++;
Hmm, actually... it should be the opposite, otherwise it wouldn't work as
intended.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3417#discussion_r1824138164
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
@dmnks commented on this pull request.
> @@ -412,3 +415,43 @@ void rpmlog (int code, const char *fmt, ...)
exit:
errno = saved_errno;
}
+
+int rpmlogOnce (uint64_t domain, const char * key, int code, const char *fmt,
...)
+{
+int saved_errno = errno;
+rpmlogCtx ctx = rpmlogCtxAcquire();
+int newkey = 0;
+
+if (ctx) {
+ wrlock lock(ctx->mutex);
+ newkey = !ctx->seen[domain][{code, key}]++;
Just proves Panu's point of this being perhaps a bit confusing :smile:
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3417#discussion_r1824147547
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
@dmnks commented on this pull request.
> @@ -412,3 +415,43 @@ void rpmlog (int code, const char *fmt, ...)
exit:
errno = saved_errno;
}
+
+int rpmlogOnce (uint64_t domain, const char * key, int code, const char *fmt,
...)
+{
+int saved_errno = errno;
+rpmlogCtx ctx = rpmlogCtxAcquire();
+int newkey = 0;
+
+if (ctx) {
+ wrlock lock(ctx->mutex);
+ newkey = !ctx->seen[domain][{code, key}]++;
Actually now that I think about it, this follows from the definition of the ++
operator.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3417#discussion_r1824148651
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
And yup, for reasons like this it's best done as an internal-only API just now - so we don't need to commit to it for the rest of our lives :sweat_smile: -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/3417#issuecomment-2449386312 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
Yeah the domain and its attached seen-stuff could be tracked as a proper object too, I mumbled something along those lines in #3389 actually, the id is just a kind of simplified version of that. The log context itself is still a kind of different thing because at least in rpm, if you specify a transaction log context, then what other output is there during a transaction? Ditto for a package build? And they'd all probably be outputing to the same stderr/file anyhow. Maybe there's a case, it's entirely possible, but we really cannot get into *that* business now. It'll need to be thought through once we decide to do a logging overhaul. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/3417#issuecomment-2449383891 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
> Redoing the whole rpmlog thing is a story for another time. [...] > but we really cannot get into that business now. It'll need to be thought > through once we decide to do a logging overhaul. Agreed, all are good points. Let's go ahead with this. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/3417#issuecomment-2449393522 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
@pmatilai commented on this pull request.
> @@ -412,3 +415,43 @@ void rpmlog (int code, const char *fmt, ...)
exit:
errno = saved_errno;
}
+
+int rpmlogOnce (uint64_t domain, const char * key, int code, const char *fmt,
...)
+{
+int saved_errno = errno;
+rpmlogCtx ctx = rpmlogCtxAcquire();
+int newkey = 0;
+
+if (ctx) {
+ wrlock lock(ctx->mutex);
+ newkey = !ctx->seen[domain][{code, key}]++;
"This" is unclear :sweat_smile: Maybe something to the tune of "[] access on
map creates the element on first access", but the more you think about it the
more the comment starts seeming redundant :sweat_smile:
So dunno - up to you.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3417#discussion_r1824130401
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
I don't disagree that this can probably be done better with proper OO. But I don't want to get into this right now. Redoing the whole rpmlog thing is a story for another time. As this is a internal API for now we can still change it later one when we get to that. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/3417#issuecomment-2449381162 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
@dmnks commented on this pull request.
> @@ -412,3 +415,43 @@ void rpmlog (int code, const char *fmt, ...)
exit:
errno = saved_errno;
}
+
+int rpmlogOnce (uint64_t domain, const char * key, int code, const char *fmt,
...)
+{
+int saved_errno = errno;
+rpmlogCtx ctx = rpmlogCtxAcquire();
+int newkey = 0;
+
+if (ctx) {
+ wrlock lock(ctx->mutex);
+ newkey = !ctx->seen[domain][{code, key}]++;
For me, the most confusing part is the increment vs. negation, it's not clear
which one takes precedence (without looking it up in the C++ docs). I assume
it's first incremented and only then negated, right?
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3417#discussion_r1824137367
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
@ffesti commented on this pull request.
> + }
+ va_end(ap);
+}
+errno = saved_errno;
+return newkey;
+}
+
+void rpmlogReset(uint64_t domain, int mode=0)
+{
+rpmlogCtx ctx = rpmlogCtxAcquire();
+std::map, int> domain_data = {};
+
+if (ctx) {
+ wrlock lock(ctx->mutex);
+ if (mode)
+ domain_data = ctx->seen[domain];
Deleted.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3417#discussion_r1824115788
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add rpmlogOnce() and rpmlogReset() (PR #3417)
@dmnks commented on this pull request.
> @@ -412,3 +415,43 @@ void rpmlog (int code, const char *fmt, ...)
exit:
errno = saved_errno;
}
+
+int rpmlogOnce (uint64_t domain, const char * key, int code, const char *fmt,
...)
+{
+int saved_errno = errno;
+rpmlogCtx ctx = rpmlogCtxAcquire();
+int newkey = 0;
+
+if (ctx) {
+ wrlock lock(ctx->mutex);
+ newkey = !ctx->seen[domain][{code, key}]++;
No, it must be the other way around, hmm. :smile:
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3417#discussion_r1824146939
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
[Rpm-maint] [rpm-software-management/rpm] Deprecated libimaevm symbol (Issue #3419)
The `sign_hash()` function is now deprecated in `libimaevm.so.5` (shipped in Fedora 41) which causes a build warning (when configured with `-DWITH_IMAEVM=ON`: ``` warning: ‘int sign_hash(const char*, const unsigned char*, int, const char*, const char*, unsigned char*)’ is deprecated [-Wdeprecated-declarations] 56 | siglen = sign_hash(algo, fdigest, diglen, key, keypass, signature+1); | ~^~ In file included from /home/mdomonko/code/rpm/f41-check/sign/rpmsignfiles.hh:12, from /home/mdomonko/code/rpm/f41-check/sign/rpmsignfiles.cc:17: /usr/include/imaevm.h:241:23: note: declared here 241 | IMAEVM_DEPRECATED int sign_hash(const char *algo, const unsigned char *hash, | ^ ``` -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/3419 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Support Fedora 41 in Dockerfile (PR #3418)
Merged #3418 into master. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/3418#event-15035058916 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
