Scriptlet interpreter and prein, postin (using lua), and verifyscript support added #95

totph · 2024-10-08T13:52:20Z

tar2rpm: scriptlet interpreter and prein postin support

Overriding the interpreter (defaults to /bin/sh) for all or a specific
scriptlet is possible via -interpreter or -interpreter_for NAME:INTERPRETER
where NAME is the type of scriptlet, one of:
prein postin preun postun pretrans posttrans verifyscript, e.g. prein:/bin/ksh

pretrans and posttrans, which should be lua code, are now handled. Their
interpreter '<lua>' is not overwritten, unless -interpreter_for is used.

Also added verifyscript support. When calling this during rpm -V pkgname
only the stderr output will be printed.

The scriptlets of an rpm can be checked via rpm -qp --scripts RPMFILE.

Support <lua> for pretrans and posttrans scriptlets

These usually do not fork interpreters, so do not override it when calling
SetScriptletInterpreter(). Note that explicity setting /bin/sh also seems
to work, and so does setting for other scriptlet. Use at your own risk!

See https://rpm-software-management.github.io/rpm/manual/lua.html

Support setting the scriptlet interpreter

The default interpreter '/bin/sh' can be changed via SetScriptletInterpreter(),
or on a per-scriptlet basis via SetScriptletInterpreterFor()

We use the more familiar name interpreter, RPM calls it scriptlet program.

Update url pointing to rpmtag.h

jarondl · 2024-10-09T06:13:05Z

rpm.go

+}
+
+func (r *RPM) writeSingleScriptlet(h *index, scriptlet, name string, scriptletTag, scriptletProgramTag int) error {
+	if scriptlet != "" {


Prefer using an early return to avoid this indentation. i.e.

if scriptlet == "" { return nil }

jarondl · 2024-10-09T06:40:36Z

rpm.go

+func (r *RPM) writeSingleScriptlet(h *index, scriptlet, name string, scriptletTag, scriptletProgramTag int) error {
+	if scriptlet != "" {
+		scriptletProgram := r.scriptletProgram
+		// All this manual parsing instead of a simple regex to provide detailed error messages like rpmbuild:


But.. We are not rpmbuild. One of our aims is simplicity.

I really appreciate the effort you put in here, so it's not easy for me to say so, but please remove all of this verification code.

rpmbuild is a very over-engineered and complex beast, riddled with backwards-compatibility-necessitated junk. It has a ton of implicit assumptions and checks. (And a lot of other problems too).

Our implementation is simple. We don't build anything. Originally we only supported rpms that are a "bag of files". Later we consented to adding scriptles. But verifying them goes a step too far.

If the user supplies a bad script, I guess it will fail for them?

Okay, makes sense! What about going one step further and removing the -p/path/here syntax entirely? The rpmpack API could be extended to either have an additional AddPostinInterpreter (or AddPostinScriptletProgram?) for every scriptlet. Or just one SetScriptletProgramFor(what, path string) where what is postin, preun etc. Either way, in tar2rpm just a single flag for all scriptlet types in the form of
-interpreter_for "preun:/bin/foo" could be added.

Yes!
The -p thing is messy.

I think SetScriptletProgramFor makes sense. I'm not sure what you propose by the single flag - how will a user specify separate interpreters?

totph · 2024-11-01T17:02:42Z

Relevant --help output of tar2rpm:

-interpreter string
      interpreter (scriptlet program) to run scriptlets with (default "/bin/sh")
-interpreter_for NAME:INTERPRETER
      override interpreter for a scriptlet, format NAME:INTERPRETER, where NAME is one of
      prein postin preun postun verifyscript, e.g. prein:/bin/bash
      [+]: pretrans and posttrans use '<lua>' by default, which is not changed by -interpreter

[..]

-postin string
      postin scriptlet contents (not filename).
-posttrans string
      posttrans scriptlet contents (not filename, lua code [+])
-postun string
      postun scriptlet contents (not filename)
-prefixes string
      comma separated prefixes for relocatable packages
-prein string
      prein scriptlet contents (not filename).
-pretrans string
      pretrans scriptlet contents (not filename, lua code [+])

totph · 2024-11-01T17:07:20Z

rpm.go

+		files:                make(map[string]RPMFile),
+		customTags:           make(map[int]IndexEntry),
+		customSigs:           make(map[int]IndexEntry),
+		scriptletInterpreter: DefaultScriptletInterpreter,


scriptletInterpreter can be removed, it always references DefaultScriptletInterpreter now.

(Now used again, so SetScriptletInterpreterFor() is not overwritten by a later SetDefaultScriptletInterpreter())

jarondl

Thank you very much for the contribution! And for your patience.

I think I spotted a surprising behavior when setting the overall interpreter (there is a comment there), but this points out the fact that there are no tests. Can you add unit tests for the basic behavior? (Adding an interpreter, setting content, etc)?

Bonus points (imaginary of-course) if you add an actual test that uses an rpm based linux, in the "example_bazel" folder. This is a bit harder, but take a look at the examples and see if something works for you. It's harder because you have to install bazelisk and docker to make it work, so I'll understand if you skip it.

/gcbrun

jarondl · 2024-11-04T06:52:24Z

rpm.go

+	interpreter string
+}
+
+type ScriptletType struct {


Should this be exported? Or should it be scriptletType?

jarondl · 2024-11-04T06:54:43Z

rpm.go

+	scriptlets           map[string]Scriptlet
+}
+
+type Scriptlet struct {


This is not directly used by end users, so it should be unexported.

jarondl · 2024-11-04T06:55:49Z

rpm.go

+}
+
+type Scriptlet struct {
+	// User provided. If `interpreter` is empty, `DefaultScriptletInterpreter` is used.


"User provided" confused me more then it helped. 90% of the values in the rpm struct can be said to be user provided one way or another. Remove it.

jarondl · 2024-11-04T06:57:08Z

rpm.go

-// AddPretrans adds a pretrans scriptlet
+func (r *RPM) writeScriptlets(h *index) {
+	for name, s := range r.scriptlets {
+		if s.content != "" || s.interpreter != "" {


Prefer the inverse logic to reduce indentation (aka guard clause)

if s.content == "" && s.interpreter == "" { continue }

jarondl · 2024-11-04T06:59:23Z

rpm.go

+			if s.interpreter == "" {
+				interpreter := r.scriptletInterpreter
+				if tagInfo.lua {
+					interpreter = "<lua>"


Is <lua> really how it's written in RPM? Crazy.

There's also lua macros in .spec files which are evaluated at rpm build time!

jarondl · 2024-11-04T07:02:39Z

rpm.go

+			if s.content != "" {
+				h.Add(tagInfo.tag, EntryString(s.content))
+			}
+			if s.interpreter == "" {


I think the "positive" logic would easier to follow here. Also avoid the extra indentation by using an else if.

"if the interpreter is not empty, use it. Else if it's lua, then use lua. Else use the default"

Done, well, moved around. But removed the indentation.

jarondl · 2024-11-04T07:06:48Z

rpm.go

+// all scriptlets, defaults to `/bin/sh`. Will reset anything set by `SetScriptletInterpreterFor()`.
+// An emtpy string resets to the default. pretrans and posttrans usually use <lua> and are skipped.
+// The scriptlets of an rpm can be checked via `rpm -qp --scripts RPMFILE`.
+func (r *RPM) SetScriptletInterpreter(interpreter string) {


I'm somewhat troubled by this new logic. Imagine this: Someone adds a preinstall script, and uses SetScriptletInterpreter to use bash instead of sh.

AFAICT, this used to work as expected - only a preinstall script is added, and it uses bash. I think that with your new logic, this will set "interpreter" for all of the scripts, and will call them emptily for all of them.

Shouldn't this change the scriptletInterpreter value?

Indeed, now using DefaultScriptletInterpreter for that again. Also, now an explicit SetScriptletInterpreterFor() has a higher precedence than a later SetDefaultScriptletInterpreter(), and added Default to the name.

jarondl · 2024-11-04T07:17:40Z

rpm.go

+	if !found {
+		return errors.New(fmt.Sprintf("invalid scriptlet name %q", name))
+	}
+	content := ""


Use the magic of zero values in go to avoid someof this:

item := r.scriptlets[name] item.interpreter = interpreter r.scriptlets[name] = item

Should just work, correctly, no matter if the value existed before. Here, I tested it out for you:
https://go.dev/play/p/ehTutXq-MwD

Done. Not sure I like this magic :)

jarondl · 2024-11-04T07:18:11Z

rpm.go

+	_, found := scriptletsTypes[name]
+	if !found {
+		panic(fmt.Sprintf("internal error: invalid scriptlet name %q", name))
+	}


Same comment about zero value.

jarondl · 2024-11-04T07:25:38Z

/gcbrun

The default interpreter '/bin/sh' can be changed via `SetDefaultScriptletInterpreter()`, or on a per-scriptlet basis via `SetScriptletInterpreterFor()` We use the more familiar name `interpreter`, RPM calls it `scriptlet program`.

These usually do not fork interpreters, so do not override them when calling `SetDefaultScriptletInterpreter()`. Note that explicity setting /bin/sh also *seems* to work, and so does setting <lua> for other scriptlet. Use at your own risk! See <https://rpm-software-management.github.io/rpm/manual/lua.html>

Overriding the interpreter (defaults to /bin/sh) for all or a specific scriptlet is possible via `-interpreter` or `-interpreter_for NAME:INTERPRETER` where NAME is the type of scriptlet, one of: prein postin preun postun pretrans posttrans verifyscript, e.g. `prein:/bin/ksh` pretrans and posttrans, which should be lua code, are now handled. Their interpreter '<lua>' is not overwritten, unless -interpreter_for is used. Also added verifyscript support. When calling this during `rpm -V pkgname` only the stderr output will be printed. The scriptlets of an rpm can be checked via `rpm -qp --scripts RPMFILE`.

totph · 2025-02-07T16:50:51Z

unit tests

Added, now writeScriptlets() does less and implicitToExplicitScriptlets() has most of the logic.

an actual test that uses an rpm based linux

I already have bazelisk and docker, the bazel-go-docker workflow isn't quite clear to me. Can you provide a basic test which builds an rpm with scriptlets and makes side effects of installing, calling rpm -V etc. visible? And to test the "interpreter only, no script content", maybe a go program visible to the test which just echos "$@" into /tmp/hello when it runs?

totph force-pushed the scriptlet_programs branch from 5b5cdca to 414daf4 Compare October 8, 2024 13:56

jarondl reviewed Oct 9, 2024

View reviewed changes

totph force-pushed the scriptlet_programs branch from 414daf4 to d21e2ab Compare November 1, 2024 16:56

totph changed the title ~~Support scriptlet programs and '-p/my/prog' scriptlets~~ Scriptlet interpreter and prein, postin (using lua), and verifyscript support added Nov 1, 2024

totph commented Nov 1, 2024

View reviewed changes

jarondl reviewed Nov 4, 2024

View reviewed changes

totph added 5 commits February 7, 2025 16:48

Update url pointing to rpmtag.h

c386a52

Support setting the scriptlet interpreter

85cdbc2

The default interpreter '/bin/sh' can be changed via `SetDefaultScriptletInterpreter()`, or on a per-scriptlet basis via `SetScriptletInterpreterFor()` We use the more familiar name `interpreter`, RPM calls it `scriptlet program`.

Add scriptlet tests

673ce66

totph force-pushed the scriptlet_programs branch from d21e2ab to a2cd292 Compare February 7, 2025 16:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Scriptlet interpreter and prein, postin (using lua), and verifyscript support added #95

Scriptlet interpreter and prein, postin (using lua), and verifyscript support added #95

totph commented Oct 8, 2024 •

edited

Loading

jarondl Oct 9, 2024

totph Oct 10, 2024

jarondl Oct 9, 2024

totph Oct 10, 2024 •

edited

Loading

jarondl Oct 21, 2024

totph Nov 1, 2024

totph commented Nov 1, 2024

totph Nov 1, 2024

totph Feb 7, 2025

jarondl left a comment

jarondl Nov 4, 2024

totph Feb 7, 2025

jarondl Nov 4, 2024

totph Feb 7, 2025

jarondl Nov 4, 2024

totph Feb 7, 2025

jarondl Nov 4, 2024

totph Feb 7, 2025

jarondl Nov 4, 2024

totph Feb 7, 2025

jarondl Nov 4, 2024

totph Feb 7, 2025

jarondl Nov 4, 2024

totph Feb 7, 2025

jarondl Nov 4, 2024

totph Feb 7, 2025

jarondl Nov 4, 2024

totph Feb 7, 2025

jarondl commented Nov 4, 2024

totph commented Feb 7, 2025

Scriptlet interpreter and prein, postin (using lua), and verifyscript support added #95

Are you sure you want to change the base?

Scriptlet interpreter and prein, postin (using lua), and verifyscript support added #95

Conversation

totph commented Oct 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

totph Oct 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

totph commented Nov 1, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jarondl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jarondl commented Nov 4, 2024

totph commented Feb 7, 2025

totph commented Oct 8, 2024 •

edited

Loading

totph Oct 10, 2024 •

edited

Loading