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

Argument in %triggerprein not consistent with %prein #2868

Open
dmnks opened this issue Jan 22, 2024 · 0 comments
Open

Argument in %triggerprein not consistent with %prein #2868

dmnks opened this issue Jan 22, 2024 · 0 comments
Labels

Comments

@dmnks
Copy link
Contributor

dmnks commented Jan 22, 2024

A %triggerprein scriptlet receives 1 as the first argument ($1) when the package containing the trigger is upgraded. This is inconsistent with %prein which receives 2 in such a case.

This is because, for triggers, handleOneTrigger() applies psm->countCorrection to the package count when computing the first argument, whereas in regular scriptlets (e.g. %prein) we use psm->scriptArg which is initialized to 2 in rpmpsmNew().

The fix should be as simple as passing psm->scriptArg to handleOneTrigger() and using that as $1 instead of computing it from scratch.

@dmnks dmnks added the bug label Jan 22, 2024
@dmnks dmnks added this to RPM Jan 22, 2024
@github-project-automation github-project-automation bot moved this to Backlog in RPM Jan 22, 2024
@dmnks dmnks moved this from Backlog to Todo in RPM Jan 22, 2024
@dmnks dmnks removed this from RPM Jan 22, 2024
dmnks added a commit to dmnks/rpm that referenced this issue Jan 22, 2024
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
dmnks added a commit to dmnks/rpm that referenced this issue Jan 22, 2024
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
dmnks added a commit to dmnks/rpm that referenced this issue Jan 22, 2024
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
dmnks added a commit to dmnks/rpm that referenced this issue Jan 22, 2024
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
dmnks added a commit to dmnks/rpm that referenced this issue Jan 22, 2024
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
dmnks added a commit to dmnks/rpm that referenced this issue Jan 22, 2024
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
dmnks added a commit to dmnks/rpm that referenced this issue Jan 22, 2024
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
dmnks added a commit to dmnks/rpm that referenced this issue Jan 22, 2024
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
dmnks added a commit to dmnks/rpm that referenced this issue Jan 22, 2024
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
dmnks added a commit to dmnks/rpm that referenced this issue Jan 22, 2024
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
dmnks added a commit to dmnks/rpm that referenced this issue Jan 22, 2024
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
dmnks added a commit to dmnks/rpm that referenced this issue Jan 22, 2024
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
dmnks added a commit to dmnks/rpm that referenced this issue Jan 22, 2024
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
dmnks added a commit to dmnks/rpm that referenced this issue Jan 22, 2024
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
dmnks added a commit to dmnks/rpm that referenced this issue Jan 22, 2024
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
pmatilai pushed a commit that referenced this issue Jan 23, 2024
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
dmnks added a commit to dmnks/rpm that referenced this issue Jan 23, 2024
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
@dmnks dmnks added this to RPM Feb 7, 2024
@github-project-automation github-project-automation bot moved this to Backlog in RPM Feb 7, 2024
@dmnks dmnks moved this from Backlog to Todo in RPM Feb 7, 2024
@pmatilai pmatilai moved this from Todo to Priority in RPM Jun 20, 2024
@pmatilai pmatilai moved this from Priority to Todo in RPM Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Todo
Development

No branches or pull requests

1 participant