Add a test for extfs's rpm helper

[mooffie]

I'm attaching a proposed test for the rpm helper.

There are two components here:

• An independent tool, rpm2tags, which converts an RPM package to a parsable text file (a serialized hashtable). This lets our tests run even where the 'rpm' program isn't installed (I assume this is an objective of ours; maybe I'm wrong).

• A code snippet that gets injected into the helper and imitates the 'rpm' program.

There's documentation in the commit messages and in the code itself so I'll stop here.

[mooffie]

Now, an obvious question is: "is this the right approach?"

A very cursory inspection of the patches may give an impression there's relatively too much code for too little. But a more careful inspection shows that:

• The heart of rpm2tags is just 5 lines of code.

• The injected shell snippet: its code is straightforward and no evisceration of the main code was needed.

The assumption was that we want tests to run without the 'rpm' binary. Even if we removed this condition, we'd still need some code, and documentation, and I'm not too sure we'd end up needing substantially less than in the proposed test.

In short: I myself am in favor of the proposed test.

[mooffie]

(In #3750) (Of course, if #3751 is rejected this one gets unblocked.)

[jtyr]

You can use this spec file to build an RPM package with all possible flags:

​https://gist.github.com/jtyr/b9d6abd0f2ad2c17daa75b02c8278d13

[mooffie]

Replying to jtyr:

You can use this spec file to build an RPM package with all possible flags:

Thanks! it works.

(I'd better wait for Yury / Andrew to opine on this whole thing before I work on it further.)

[zaytsev]

I've had a look at the code, and to me it looks great! I've noticed a typo in mcrpmfs_getDesription, btw, but then I realized it's likewise mistyped in the helper :-) That feeling when I wanted to criticize the injected script, and then realized I got it wrong...

Branch: 3751_extfs_rpm_tests
Initial changeset: 417917a324b43bf9cdeb41f88198881c2215f015

[andrew_b]

What reason to place rpm2tags into helpers-list/misc subdirectory instead of directly in helpers-list/?
rpm2tags should be rpm2tags.in and use @PERL@ in shebang instead of direct /usr/bin/perl.
Probably, rpm.rewrite.sh also should use @PERL@ instead of perl.
rpm.rewrite.sh should be executable.

[mooffie]

@Andrew: I'll reply to your comment later today. I didn't know about it before I prepared the following patch.

I'm attaching a patch that adds a test for @jtyr's contributed test.spec package.

(The next patch I intended to publish was to move the two files from 'misc/' to 'misc/rpm/', for the sake of tidyness. BTW, it's my plan someday to place each helper's tests in a separate folder, and when this day comes we'll get rid of the 'misc/rpm' folder by moving it there. However, since @Andrew wants me to modify or rename 'misc/rpm2tags', I'll have to delay that patch as it touches this path.)

[mooffie]

Replying to andrew_b:

What reason to place rpm2tags into helpers-list/misc subdirectory instead of directly in helpers-list/?

• It doesn't belong among "normal" files: it's an auxiliary tool: it's not run by any of MC's build / test / maintenance procedures.

• It's not part of the test harness. It's out-of-place in that directory in this regard too.

• I could have considered putting in helpers-list/ auxiliary tools that are relevant for all the helpers, but this one is specific to the rpm one. Putting it there will make helpers-list/ a cluttered "catch all" folder.

Ditto for @jtyr's contributed file, 'test.spec'. In my recent patch I placed it in that same 'misc' folder (which I intend to rename to 'misc/rpm'). I'm sure you'll agree that putting it too in helpers-list/ is inelegant.

Maybe I should have named that directory 'maint' instead of 'misc'? Would that have made the purpose of the files within clearer?

rpm2tags should be rpm2tags.in and use @PERL@ in shebang instead of direct /usr/bin/perl.

I agree that hardcoding '/usr/bin/perl' is not ok.

If you want me to have 'rpm2tags.in', just say so and I'll do it.

But I'd like to suggest an alternative: to use "#/usr/bin/env perl" instead. rpm2tags' use and audience is very different than the helpers' use and audience: it's supposed to be run, by hand, only by people creating new RPM tests. Why would this crowd have perl somewhere else than in PATH?

Probably, rpm.rewrite.sh also should use @PERL@ instead of perl.

Ok, I'll have to think of ways to solve this.

Let me think aloud about having 'rpm.rewrite.sh.in':

Cons:

1. It's a bother working with *.in files.
2. I'll need to tell the tester where the build tree is, so that it will be able to export, say,$MC_TEST_DATA_BUILD_DIR. Not that it's an issue, but I like the tests' current blessed ignorance of this matter.

Pros:

1. In the future we may need other *.in files. Maybe not, maybe yes. So let's do it sooner instead of later.

An alternative solution is to rename the helper from 'rpm' to 'rpm.in' and to put @PERL@ there, which 'rpm.rewrite.sh' will then see.

Any thoughts, folks?

rpm.rewrite.sh should be executable.

No, it gets "sourced" into the helper (do "help ." in bash). (The ".sh" extension is there only for the sake of documentation, and to help editors detect the syntax.)

[mooffie]

BTW, could anybody explain the rationale behind @PERL@ to me?

(Once I understand the rationale I may have another question: "does this rationale apply also for 'make check'?")

[mooffie]

Ok, here are two patches to handle the perl path issue (this first one is a "fixup").

[mooffie]

Finally, a patch to move misc/* to misc/rpm/*.

[zaytsev]

BTW, could anybody explain the rationale behind @PERL@ to me?

The main reason for shipping scripts (Perl, Python, Ruby) etc. as *.in files that get their shebangs expanded with the interpreter path is that the Linux default of "/usr/bin/xxx" doesn't apply to a whole host of Unix systems, as in BSDs like FreeBSD, SysV descendants like Solaris, and so on. In those cases "/usr/local/bin/perl" is the least surprising of all you can get, but think of Solaris (if you get your Perl from OpenCSW, it's "/opt/csw/bin/perl", whereas SunFreeware Perl goes to "/usr/local/bin/perl", and the list goes on), AIX, HP-UX and such.

Now the question is what's the problem with using /usr/bin/env? The answer is more subtle, and the point here is that distro maintainers totally hate it for package manager installed software (i.e. on BSD the ports system, or OpenCSW on Solaris), because it makes the behavior of such software unpredictable. As a distro packager, the only way of staying halfway sane is to be sure that interpreters for scripts shipped with your packages are picked from your supported pool of interpreters you build the software against.

Why is this important? Because often packaged software depends on the interpreter internals, e.g. ABIs for modules, like Python extensions, or some such things that change from version to version and are then autodiscovered at build time. But even if this is NOT the case, consider program FOO written in Python 2, which has #!/usr/bin/env python as a shebang and it's installed system-wide from a package manager. User A is not creative and is just using it normally. User B installed a custom built Python 3 interpreter in his $HOME/bin and because he wants Python 3 by default and is exceedingly stupid, he symlinked python3 -> python. Of course, all system-wide software using /usr/bin/env python is broken now and litters his console with exceptions. He calls up his friendly admin team explaining how badly the systems are set up. Of course, he rejects the notion that he has no clue about Unix and could have possibly done something wrong. A heated exchange ensues to the detriment of collegial relationships.

The way to avoid such problems (much more common that you would think in various forms, including the scenario above) is to hardcode reference interpreter in all packaged software.

Now, I've explained all this to clarify the background why certain things are done in certain ways. I'm not actually sure that the second part about /usr/bin/env applies to our case. The rpm2tags script is a maintainer thing, and I think using it there is completely safe. The tester things is more of a borderline case, but I can see the argument why one could use /usr/bin/env there as well: one can claim that tests are run on a reference system anyways, and thus one shouldn't expect any surprises there, and when they are run by a user, because the software was built from source, @PERL@ would resolve to his personal interpreter anyways. The @PERL@ thing is important for things like extfs scripts, but here, maybe /usr/bin/env can be used instead to not to mess with the substitutions. I haven't made up my mind and I need to go. What do you guys think?

[andrew_b]

I think we should use the @XXX@ pattern for each/any program found via AC_PATH_PROG.

[mooffie]

@Yury: thanks a bunch! that explanation was very helpful.

As for rpm2tags: I'll make it use @PERL@. I don't think there's sense in this, but I want this ticket to move forward.

As for 'rpm.rewrite.sh' or any other script in the 'data' folder: it's easy to implement a mechanism to give us access to @PERL@ et al even in non *.in files. I'll publish a patch (in a separate ticket) next week. So, while my 3751-0006 patch already solved this issue, I'm now retracting that patch in favor of the (yet-to-be-posted) new mechanism.

So, to sum it up:

My last three attachments are canceled. (Is there a way to draw a strikethrough line, or other marking, over attachments here? I don't want to delete these files because they are still informative.)

[mooffie]

I'll resume working on this as soon as #3753 gets in.

As for rpm2tags: I'll make it use @PERL@. I don't think there's sense in this, but I want this ticket to move forward.

No, I've changed my mind: I'll simply rename it to rpm2tags.pl. Then we won't need the shebang. Why? Because otherwise I'll need to use that same uncomfortable xxx.in scheme for any other maintenance scripts we may add.

[mooffie]

Ok, I've pushed an updated branch. It addresses the perl issue.

Branch: 3751_extfs_rpm_tests
Initial changeset: 76e4bcdd7c910e8377bc4ea23a17bc67df9bd492

This new branch also:

• Adds a test for @jtyr's custom test.rpm (in addition to glib.rpm).
• Moves misc/* to misc/rpm/*.

[mooffie]

I'm attaching a 'diff' showing how exactly the perl issue was solved. In short:

• 'rpm.rewrite.sh' now uses $PERL (thanks to #3753).
• rpm2tags was renamed to rpm2tags.pl (and shebang line removed).

[zaytsev]

LGTM, Andrew?

[mooffie]

Merged to master: [91ca170e1b89fad91881f283a16df89309db94a8]