-
Notifications
You must be signed in to change notification settings - Fork 359
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RFE: file trigger scriptlet arguments #2755
Comments
I wonder if we want the triggering package count ( There's a subtle difference in the semantics between regular triggers and file triggers in the way they are fired. A regular trigger can only ever be triggered by one package (e.g. In both cases, the trigger only executes once, however the semantics of "which package triggered it" differs. Therefore, for regular triggers, As an example, consider the following scenario:
If However, if My take: The count should be an aggregate since that enables a file trigger script to detect when all the files subject to the prefix are being removed (which is one of the purposes of the arguments, also for regular triggers, in the first place). Any thoughts? |
Aggregate is what I thought of when writing the description, I don't see anything else making much sense. But, it's not like I've given this any deep meditation. Usefulness is all that matters for the arguments, otherwise we could just as well not bother 😅 |
Thanks, the part about usefulness is actually on point 😄 That said, I haven't given it too much thought either, this is just the most obvious solution that comes to mind. I'll ponder about it a bit more but there's probably not much to ponder about anyway. |
The thing to ponder about is whether there are other arguments that should be passed in addition or in stead of these. The only "vision" or "design" behind this ticket description is "something close enough to other scriptlets that someone familiar with should feel at home". Which may leave some room for improvement from a more technical perspective 😆 |
Good point! We shouldn't just blindly copy what the normal triggers do here. There's enough difference that it makes sense to step back for a moment and think about what functionality and use cases we really want to cover. Thanks, I'll take that into account 😄 |
Note that there's no technical reason for not adding the second argument (the number of triggering packages) to transaction scriplets here, too. It's the same code path underneath. Whether it's useful or not is another question but I'll probably lean towards consistency here and include it. |
Pass the number of instances of the source (i.e. triggered) package left after the operation as the first argument ($1) to file trigger scripts, similarly to regular triggers. The %filetrigger{in,un,postun} variants execute at the same stages as their regular trigger counterparts so use the same logic to compute the argument. The %transfiletrigger{in,un} variants don't have any regular trigger counterparts but are close relatives of the %preuntrans and %postuntrans scriptlets, respectively, so borrow the logic from those. Don't count packages in runImmedFileTriggers() again, though, and just reuse the psm->scriptArg value which is already computed in rpmpsmNew() and used for the normal scriptlets. We could/should probably do this in runImmedTriggers(), too, but leave that for later (see issue rpm-software-management#2868). Some tests already cover the file trigger arguments so adjust those. To bring the file trigger tests on par with the regular trigger ones, extend the "basic file triggers 2" test with the remaining scenarios. This is easier than extending "basic file trigger scripts" since we don't really need to test the filenames returned on stdin here, just the arguments. Related: rpm-software-management#2755
Pass the number of instances of the source (i.e. triggered) package left after the operation as the first argument ($1) to file trigger scripts, similarly to regular triggers. The %filetrigger{in,un,postun} variants execute at the same stages as their regular trigger counterparts so use the same logic to compute the argument. The %transfiletrigger{in,un} variants don't have any regular trigger counterparts but are close relatives of the %preuntrans and %postuntrans scriptlets, respectively, so borrow the logic from those. Don't count packages in runImmedFileTriggers() again, though, and just reuse the psm->scriptArg value which is already computed in rpmpsmNew() and used for the normal scriptlets. We could/should probably do this in runImmedTriggers(), too, but leave that for later (see issue rpm-software-management#2868). Some tests already cover the file trigger arguments so adjust those and extend the "basic file triggers 2" test with the remaining scenarios to bring it on par with the regular trigger tests. This is easier than extending "basic file trigger scripts" since we don't really need to test the filenames returned on stdin here, just the arguments. Related: rpm-software-management#2755
Pass the number of instances of the source (i.e. triggered) package left after the operation as the first argument ($1) to file trigger scripts, similarly to regular triggers. The %filetrigger{in,un,postun} variants execute at the same stages as their regular trigger counterparts so use the same logic to compute the argument. The %transfiletrigger{in,un} variants don't have any regular trigger counterparts but are close relatives of the %preuntrans and %postuntrans scriptlets, respectively, so borrow the logic from those. Don't count packages in runImmedFileTriggers() again, though, and just reuse the psm->scriptArg value which is already computed in rpmpsmNew() and used for the normal scriptlets. We could/should probably do this in runImmedTriggers(), too, but leave that for later (see issue rpm-software-management#2868). Some tests already cover the file trigger arguments so adjust those, and extend the "basic file triggers 2" test with the remaining scenarios to bring it on par with the regular trigger tests. This is easier than extending "basic file trigger scripts" since we don't really need to test the filenames returned on stdin here, just the arguments. Related: rpm-software-management#2755
Pass the number of instances of the source (i.e. triggered) package left after the operation as the first argument ($1) to file trigger scripts, similarly to regular triggers. The %filetrigger{in,un,postun} variants execute at the same stages as their regular trigger counterparts so use the same logic to compute the argument. The %transfiletrigger{in,un} variants don't have any regular trigger counterparts but are close relatives of the %preuntrans and %postuntrans scriptlets, respectively, so borrow the logic from those. Don't count packages in runImmedFileTriggers() again, though, and just reuse the psm->scriptArg value which is already computed in rpmpsmNew() and used for the normal scriptlets. We could/should probably do this in runImmedTriggers(), too, but leave that for later (see issue rpm-software-management#2868). Some tests already cover the file trigger arguments so adjust those, and extend the "basic file triggers 2" test with the remaining scenarios to bring it on par with the regular trigger tests. This is easier than extending "basic file trigger scripts" since we don't really need to test the filenames returned on stdin here, just the arguments. Also add a short note about the argument to the file trigger docs. Related: rpm-software-management#2755
Pass the number of instances of the source (i.e. triggered) package left after the operation as the first argument ($1) to file trigger scripts, similarly to regular triggers. The %filetrigger{in,un,postun} variants execute at the same time as their regular trigger counterparts so use the same logic to compute the argument. The %transfiletrigger{in,un} variants don't have any regular trigger counterparts but are close relatives of the %preuntrans and %postuntrans scriptlets, respectively, so borrow the logic from those. Don't count packages in runImmedFileTriggers() again, though, and just reuse the psm->scriptArg value which is already computed in rpmpsmNew() and used for the normal scriptlets. We could/should probably do this in runImmedTriggers(), too, but leave that for later (see issue rpm-software-management#2868). Some tests already cover the file trigger arguments so adjust those, and extend the "basic file triggers 2" test with the remaining scenarios to bring it on par with the regular trigger tests. This is easier than extending "basic file trigger scripts" since we don't really need to test the filenames returned on stdin here, just the arguments. Also add a short note about the argument to the file trigger docs. Related: rpm-software-management#2755
Pass the number of instances of the source (i.e. triggered) package left after the operation as the first argument ($1) to file trigger scripts, similarly to regular triggers. The %filetrigger{in,un,postun} variants execute at the same time as their regular trigger counterparts so use the same logic to compute the argument. The %transfiletrigger{in,un} variants don't have any regular trigger counterparts but are close relatives of the %postuntrans and %preuntrans scriptlets, respectively, so borrow the logic from those. Don't count packages in runImmedFileTriggers() again, though, and just reuse the psm->scriptArg value which is already computed in rpmpsmNew() and used for the normal scriptlets. We could/should probably do this in runImmedTriggers(), too, but leave that for later (see issue rpm-software-management#2868). Some tests already cover the file trigger arguments so adjust those, and extend the "basic file triggers 2" test with the remaining scenarios to bring it on par with the regular trigger tests. This is easier than extending "basic file trigger scripts" since we don't really need to test the filenames returned on stdin here, just the arguments. Also add a short note about the argument to the file trigger docs. Related: rpm-software-management#2755
Pass the number of instances of the source (i.e. triggered) package left after the operation as the first argument ($1) to file trigger scripts, similarly to regular triggers. The %filetrigger{in,un,postun} variants execute at the same time as their regular trigger counterparts so use the same logic to compute the argument. The %transfiletrigger{in,un} variants don't have any regular trigger counterparts but are close relatives of the %posttrans and %preuntrans scriptlets, respectively, so borrow the logic from those. Don't count packages in runImmedFileTriggers() again, though, and just reuse the psm->scriptArg value which is already computed in rpmpsmNew() and used for the normal scriptlets. We could/should probably do this in runImmedTriggers(), too, but leave that for later (see issue rpm-software-management#2868). Some tests already cover the file trigger arguments so adjust those, and extend the "basic file triggers 2" test with the remaining scenarios to bring it on par with the regular trigger tests. This is easier than extending "basic file trigger scripts" since we don't really need to test the filenames returned on stdin here, just the arguments. Also add a short note about the argument to the file trigger docs. Related: rpm-software-management#2755
Pass the number of instances of the source (i.e. triggered) package left after the operation as the first argument ($1) to file trigger scripts, similarly to regular triggers. The %filetrigger{in,un,postun} variants execute at the same time as their regular trigger counterparts so use the same logic to compute the argument. The %transfiletrigger{in,un} variants don't have any regular trigger counterparts but are close relatives of the %posttrans and %preuntrans scriptlets, respectively, so borrow the logic from those. Don't count packages in runImmedFileTriggers() again, though, and just reuse the psm->scriptArg value which is already computed in rpmpsmNew() and used for the normal scriptlets such as %pre. We could/should probably do this in runImmedTriggers(), too, but leave that for later (see issue rpm-software-management#2868). Some tests already cover the file trigger arguments so adjust those, and extend the "basic file triggers 2" test with the remaining scenarios to bring it on par with the regular trigger tests. This is easier than extending "basic file trigger scripts" since we don't really need to test the filenames returned on stdin here, just the arguments. Also add a short note about the argument to the file trigger docs. Related: rpm-software-management#2755
Pass the number of instances of the source (i.e. triggered) package left after the operation as the first argument ($1) to file trigger scripts, similarly to regular triggers. The %filetrigger{in,un,postun} variants execute at the same time as their regular trigger counterparts so use the same logic to compute the argument. The %transfiletrigger{in,un} variants don't have any regular trigger counterparts but are close relatives of the %posttrans and %preuntrans scriptlets, respectively, so borrow the logic from those. Don't count packages in runImmedFileTriggers() again, though, and just reuse the psm->scriptArg value which is already computed in rpmpsmNew() and used for the normal scriptlets such as %pre. We could/should probably do this in runImmedTriggers(), too, but leave that for later (see issue rpm-software-management#2868). Some tests already cover the file trigger arguments so adjust those, and extend the "basic file triggers 2" test with the remaining scenarios to bring it on par with the regular trigger tests. This is easier than extending "basic file trigger scripts" since we don't really need to test the filenames returned on stdin here, just the arguments. Also add a short note about the argument to the file trigger docs. Argument no. 2 (triggering package count) will be added in a separate commit. Related: rpm-software-management#2755
Pass the number of instances of the source (i.e. triggered) package left after the operation as the first argument ($1) to file trigger scripts, similarly to regular triggers. The %filetrigger{in,un,postun} variants execute at the same time as their regular trigger counterparts so use the same logic to compute the argument. The %transfiletrigger{in,un} variants don't have any regular trigger counterparts but are close relatives of the %posttrans and %preuntrans scriptlets, respectively, so borrow the logic from those. Don't count packages in runImmedFileTriggers() again, though, and just reuse the psm->scriptArg value which is already computed in rpmpsmNew() and used for the normal scriptlets such as %pre. We could/should probably do this in runImmedTriggers(), too, but leave that for later (see issue rpm-software-management#2868). Some tests already cover the file trigger arguments so adjust those, and extend the "basic file triggers 2" test with the remaining scenarios to bring it on par with the regular trigger tests. This is easier than extending "basic file trigger scripts" since we don't really need to test the filenames returned on stdin here, just the arguments. Also add a short note about the argument to the file trigger docs. Argument no. 2 (triggering package count) will be added in a separate commit. Related: rpm-software-management#2755
Pass the number of instances of the source (i.e. triggered) package left after the operation as the first argument ($1) to file trigger scripts, similarly to regular triggers. The %filetrigger{in,un,postun} variants execute at the same time as their regular trigger counterparts so use the same logic to compute the argument. The %transfiletrigger{in,un} variants don't have any regular trigger counterparts but are close relatives of the %posttrans and %preuntrans scriptlets, respectively, so borrow the logic from those. Don't count packages in runImmedFileTriggers() again, though, and just reuse the psm->scriptArg value which is already computed in rpmpsmNew() and used for the normal scriptlets such as %pre. We could/should probably do this in runImmedTriggers(), too, but leave that for later (see issue rpm-software-management#2868). Some tests already cover the file trigger arguments so adjust those, and extend the "basic file triggers 2" test with the remaining scenarios to bring it on par with the regular trigger tests. This is easier than extending "basic file trigger scripts" since we don't really need to test the filenames returned on stdin here, just the arguments. Also add a short note about the argument to the file trigger docs. The second argument $2 (triggering package count) will be added in a separate commit. Related: rpm-software-management#2755
Pass the number of instances of the source (i.e. triggered) package left after the operation as the first argument ($1) to file trigger scripts, similarly to regular triggers. The %filetrigger{in,un,postun} variants execute at the same time as their regular trigger counterparts so use the same logic to compute the argument. The %transfiletrigger{in,un} variants don't have any regular trigger counterparts but are close relatives of the %posttrans and %preuntrans scriptlets, respectively, so borrow the logic from those. Don't count packages in runImmedFileTriggers() again, though, and just reuse the psm->scriptArg value which is already computed in rpmpsmNew() and used for the normal scriptlets such as %pre. We could/should probably do this in runImmedTriggers(), too, but leave that for later (see issue rpm-software-management#2868). Some tests already cover the file trigger arguments so adjust those, and extend the "basic file triggers 2" test with the remaining scenarios to bring it on par with the regular trigger tests. This is easier than extending "basic file trigger scripts" since we don't really need to test the filenames returned on stdin here, just the arguments. Also add a short note about the argument to the file trigger docs. The second argument $2 (triggering package count) will be added in a separate commit. Related: rpm-software-management#2755
Pass the number of instances of the source (i.e. triggered) package left after the operation as the first argument ($1) to file trigger scripts, similarly to regular triggers. The %filetrigger{in,un,postun} variants execute at the same time as their regular trigger counterparts so use the same logic to compute the argument. The %transfiletrigger{in,un} variants don't have any regular trigger counterparts but are close relatives of the %posttrans and %preuntrans scriptlets, respectively, so borrow the logic from those. Don't count packages in runImmedFileTriggers() again, though, and just reuse the psm->scriptArg value which is already computed in rpmpsmNew() and used for the normal scriptlets such as %pre. We could/should probably do this in runImmedTriggers(), too, but leave that for later (see issue rpm-software-management#2868). Some tests already cover the file trigger arguments so adjust those, and extend the "basic file triggers 2" test with the remaining scenarios to bring it on par with the regular trigger tests. This is easier than extending "basic file trigger scripts" since we don't really need to test the filenames returned on stdin here, just the arguments. Also add a short note about the argument to the file trigger docs. The second argument $2 (triggering package count) will be added in a separate commit. Related: rpm-software-management#2755
Pass the number of instances of the source (i.e. triggered) package left after the operation as the first argument ($1) to file trigger scripts, similarly to regular triggers. The %filetrigger{in,un,postun} variants execute at the same time as their regular trigger counterparts so use the same logic to compute the argument. The %transfiletrigger{in,un} variants don't have any regular trigger counterparts but are close relatives of the %posttrans and %preuntrans scriptlets, respectively, so borrow the logic from those. Don't count packages in runImmedFileTriggers() again, though, and just reuse the psm->scriptArg value which is already computed in rpmpsmNew() and used for the normal scriptlets such as %pre. We could/should probably do this in runImmedTriggers(), too, but leave that for later (see issue rpm-software-management#2868). Some tests already cover the file trigger arguments so adjust those, and extend the "basic file triggers 2" test with the remaining scenarios to bring it on par with the regular trigger tests. This is easier than extending "basic file trigger scripts" since we don't really need to test the filenames returned on stdin here, just the arguments. Also add a short note about the argument to the file trigger docs. The second argument $2 (triggering package count) will be added in a separate commit. Related: rpm-software-management#2755
Pass the number of instances of the source (i.e. triggered) package left after the operation as the first argument ($1) to file trigger scripts, similarly to regular triggers. The %filetrigger{in,un,postun} variants execute at the same time as their regular trigger counterparts so use the same logic to compute the argument. The %transfiletrigger{in,un} variants don't have any regular trigger counterparts but are close relatives of the %posttrans and %preuntrans scriptlets, respectively, so borrow the logic from those. Don't count packages in runImmedFileTriggers() again, though, and just reuse the psm->scriptArg value which is already computed in rpmpsmNew() and used for the normal scriptlets such as %pre. We could/should probably do this in runImmedTriggers(), too, but leave that for later (see issue rpm-software-management#2868). Some tests already cover the file trigger arguments so adjust those, and extend the "basic file triggers 2" test with the remaining scenarios to bring it on par with the regular trigger tests. This is easier than extending "basic file trigger scripts" since we don't really need to test the filenames returned on stdin here, just the arguments. Also add a short note about the argument to the file trigger docs. The second argument $2 (triggering package count) will be added in a separate commit. Related: rpm-software-management#2755
Pass the number of instances of the source (i.e. triggered) package left after the operation as the first argument ($1) to file trigger scripts, similarly to regular triggers. The %filetrigger{in,un,postun} variants execute at the same time as their regular trigger counterparts so use the same logic to compute the argument. The %transfiletrigger{in,un} variants don't have any regular trigger counterparts but are close relatives of the %posttrans and %preuntrans scriptlets, respectively, so borrow the logic from those. Don't count packages in runImmedFileTriggers() again, though, and just reuse the psm->scriptArg value which is already computed in rpmpsmNew() and used for the normal scriptlets such as %pre. We could/should probably do this in runImmedTriggers(), too, but leave that for later (see issue #2868). Some tests already cover the file trigger arguments so adjust those, and extend the "basic file triggers 2" test with the remaining scenarios to bring it on par with the regular trigger tests. This is easier than extending "basic file trigger scripts" since we don't really need to test the filenames returned on stdin here, just the arguments. Also add a short note about the argument to the file trigger docs. The second argument $2 (triggering package count) will be added in a separate commit. Related: #2755
Pass the number of instances of the source (i.e. triggered) package left after the operation as the first argument ($1) to file trigger scripts, similarly to regular triggers. The %filetrigger{in,un,postun} variants execute at the same time as their regular trigger counterparts so use the same logic to compute the argument. The %transfiletrigger{in,un} variants don't have any regular trigger counterparts but are close relatives of the %posttrans and %preuntrans scriptlets, respectively, so borrow the logic from those. Don't count packages in runImmedFileTriggers() again, though, and just reuse the psm->scriptArg value which is already computed in rpmpsmNew() and used for the normal scriptlets such as %pre. We could/should probably do this in runImmedTriggers(), too, but leave that for later (see issue rpm-software-management#2868). Some tests already cover the file trigger arguments so adjust those, and extend the "basic file triggers 2" test with the remaining scenarios to bring it on par with the regular trigger tests. This is easier than extending "basic file trigger scripts" since we don't really need to test the filenames returned on stdin here, just the arguments. Also add a short note about the argument to the file trigger docs. The second argument $2 (triggering package count) will be added in a separate commit. Related: rpm-software-management#2755
OK, looking closer, the challenge with transactional file triggers and package counts is that we can't easily obtain the correct (i.e. count-corrected for upgrades/removals) value for each involved package since we iterate through all files in the transaction set, as opposed to each transaction element separately (as we do with non-transactional file triggers). This is done for performance reasons (I suppose) so changing it just to add a second argument to transactional file triggers doesn't seem worth it. After all, the original requirements of this ticket explicitly mention non-trans file triggers. I guess @pmatilai knew what he was talking about when filing this ticket, after all 😄
So nope, taking it back. Let's stick with the plan and only do this for the non-trans variants. |
Pass the number of instances of the target (i.e. triggering) package left after the operation as the second argument ($2) to file trigger scripts, similarly to regular triggers. CONTINUE HERE The %transfiletrigger{in,un} variants don't have any regular trigger counterparts but are close relatives of the %posttrans and %preuntrans scriptlets, respectively, so borrow the logic from those. Also add a short note about the argument to the file trigger docs. Related: rpm-software-management#2755
Pass the number of instances of the target (i.e. triggering) package left after the operation as the second argument ($2) to file trigger scripts, similarly to regular triggers. CONTINUE HERE (explain why is arg2 disabled in transfiletrigger*) The %transfiletrigger{in,un} variants don't have any regular trigger counterparts but are close relatives of the %posttrans and %preuntrans scriptlets, respectively, so borrow the logic from those. Also add a short note about the argument to the file trigger docs. Related: rpm-software-management#2755
Pass the number of instances of the target (i.e. triggering) package left after the operation as the second argument ($2) to file trigger scripts, similarly to regular triggers. Don't pass it to the %transfiletrigger* variants, though, since we can't easily obtain a count-corrected value when such a file trigger is fired by a transaction element. This is because when runFileTriggers() is called in rpmtsRun(), no psm instance has been created yet and thus psm->scriptArg or psm->countCorrection, set by rpmpsmNew(), isn't readily available to us in runHandleTriggersInPkg(). While possible to work around, having arg2 in transaction file triggers doesn't seem worth the trouble. Make arg2 correspond to the first matching package found, as opposed to being an aggregate count of all the matching packages, even if the latter would be easier as we wouldn't have to store the matched package name in the mfi struct and just use rpmdbGetIteratorCount(mfi->pi). The reason for this is two-fold: it's consistent with how regular triggers work and it allows file triggers to detect an upgrade (arg2 > 1) of a triggering package. Fixes: rpm-software-management#2755
As for arg2 and the question of "should it be an aggregate count?", I've come to realize that it actually should not be an aggregate since that would prevent the script from detecting a package upgrade (i.e. the value of 2 or higher could either represent an upgrade or multiple preinstalled triggering packages in the rpmdb). It would also be inconsistent with regular triggers. Hence, no aggregate count in #2883. Of course, that falls apart as soon as you have multiple instances of the same package installed in parallel, but that's not a problem specific to file triggers but rather to all scripts. |
Pass the number of instances of the target (i.e. triggering) package left after the operation as the second argument ($2) to file trigger scripts, similarly to regular triggers. Don't pass it to the %transfiletrigger* variants, though, since we can't easily obtain a count-corrected value when such a file trigger is fired by a transaction element. This is because when runFileTriggers() is called in rpmtsRun(), no psm instance has been created yet and thus psm->scriptArg, set by rpmpsmNew(), isn't readily available to us in runHandleTriggersInPkg(). While possible to work around, having arg2 in transaction file triggers doesn't seem worth the trouble. Make arg2 correspond to the first matching package found, as opposed to being an aggregate count of all the matching packages, even if the latter would be easier as we wouldn't have to store the matched package name in the mfi struct and just use rpmdbGetIteratorCount(mfi->pi). The reason for this is two-fold: it's consistent with how regular triggers work and it allows file triggers to detect an upgrade (arg2 > 1) of a triggering package. Fixes: rpm-software-management#2755
Splitting this from #2655 as this is a clearly separate thing that should be doable without massive redesign of the whole thing:
The text was updated successfully, but these errors were encountered: