Skip to content
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

Pass arg2 to file trigger scripts #2883

Merged
merged 2 commits into from Feb 9, 2024

Conversation

dmnks
Copy link
Contributor

@dmnks dmnks commented Feb 2, 2024

This is the other half of #2755 adding the second argument to file trigger scripts.

No functional change, just laying the groundwork for the next commit.
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
@pmatilai
Copy link
Member

pmatilai commented Feb 6, 2024

Aside from those three-way indiretions making me cringe a bit (not that it makes any difference here), looks pretty straight-forward to me.

This is one where feedback from active packagers would be useful.

@dmnks
Copy link
Contributor Author

dmnks commented Feb 6, 2024

This is one where feedback from active packagers would be useful.

Good point! I've brought it to the related discussion thread, let's see if we get some feedback.

@pmatilai
Copy link
Member

pmatilai commented Feb 9, 2024

After mulling on it for a few days, I don't know what else we should do with this really. And, it doesn't seem to be pulling a whole lot of attention either. Let's just merge.

@pmatilai pmatilai merged commit 7f59c7d into rpm-software-management:master Feb 9, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants