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

core/scripts: Support /var/lib/rpm-state #1319

Closed
wants to merge 2 commits into from

Conversation

cgwalters
Copy link
Member

I was trying a --ex-unified-core compose of FAW, and things fell over
on urw-base35-fonts which does a dance of setting a stamp file in
%post and checking it in %posttrans.

This whole pattern should be considered deprecated by file triggers. But let's
support it for now.

Note there's a lot of parameter passing as we need a single directory which is
held across multiple script invocations.

I was trying a `--ex-unified-core` compose of FAW, and things fell over
on `urw-base35-fonts` which does a dance of setting a stamp file in
`%post` and checking it in `%posttrans`.

This whole pattern should be considered deprecated by file triggers. But let's
support it for now.

Note there's a lot of parameter passing as we need a single directory which is
held across multiple script invocations.
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks sane, just some minor nits!

&n_post_scripts_run, cancellable, error))
return FALSE;
}

/* file triggers */
if (!run_all_transfiletriggers (self, ordering_ts, tmprootfs_dfd,
if (!run_all_transfiletriggers (self, ordering_ts, tmprootfs_dfd,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: looks like a stray trailing space here.

*
* See above for why we special case glibc.
*/
const gboolean is_glibc_locales = strcmp (pkg_script, "glibc-all-langpacks.posttrans") == 0;
bwrap = rpmostree_bwrap_new (rootfs_fd,
is_glibc_locales ? RPMOSTREE_BWRAP_MUTATE_FREELY : RPMOSTREE_BWRAP_MUTATE_ROFILES,
error,
/* Scripts can see a /var with compat links like alternatives */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still true, right?

@@ -328,25 +330,43 @@ run_script_in_bwrap_container (int rootfs_fd,
else
created_var_tmp = TRUE;

/* And similarly for /var/lib/rpm-state */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about making this hunk conditional on var_lib_rpm_statedir?

@cgwalters
Copy link
Member Author

cgwalters commented Mar 28, 2018

Fixup ⬆️

@jlebon
Copy link
Member

jlebon commented Mar 28, 2018

@rh-atomic-bot r+ fb08b10

@rh-atomic-bot
Copy link

⚡ Test exempted: merge already tested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants