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

tar_git: Support for templated .spec files #5

Open
thp opened this issue Feb 16, 2015 · 3 comments
Open

tar_git: Support for templated .spec files #5

thp opened this issue Feb 16, 2015 · 3 comments

Comments

@thp
Copy link
Contributor

thp commented Feb 16, 2015

We have several projects where we build the .spec file from some template via a script (usually called rpm/precheckin.sh, but I'm sure there are others) to account for slight differences (e.g. two packages for different device adaptations that differ only in package name and build flags, or multiple variations of the same package, etc..). This is good, but has several problems:

  • We store generated files in the Git repository in addition to the template and the script
  • People forget to run precheckin.sh and patch the generated (=output) files in Git, leading to problems down the line the next time the template is updated and precheckin.sh run (I've had this at least two times, and was on the "receiving end" of this issue, meaning I had to merge back changes to generated files somebody else did into the template file)
  • Patches and pull/merge requests are hard to read, as the same changes appear multiple times (once in the template, and then once for every output file)
  • If the list of generated files changes, either precheckin.sh has to take care of removing the old file, or the developer has to figure out that the file isn't generated, and has to manually git rm the outdated generated file (e.g. an outdated device variant is removed from being generated, then the associated output file also needs to be manually removed)
  • Sometimes variants are generated from a base .spec file, making it harder to figure out which .spec file is the "template" one (e.g. precheckin.sh generates bar.spec from foo.spec via sed -e "s/FooFoo/BarBar/g" <foo.spec >bar.spec -- one has to read and understand precheckin.sh to figure out that bar.spec is read from foo.spec, and that foo.spec is used as "template" as well as "output")

I hereby propose a feature enhancement for tar_git, whereas:

  • If the .spec file is not generated, it is stored in Git like now (rpm/<packagename>.spec), nothing changes
  • If the .spec file(s) is/are (to be) generated:
    • The generated .spec files SHOULD NOT be stored in Git
    • The template file MUST NOT end in .spec (suggested: .spec.in, but not required, as it could theoretically be fully generated by rpm/precheckin.sh without any template file)
    • There MUST be a script rpm/precheckin.sh that is run by tar_git using a Bourne shell with the working directory being rpm/ (think: cd rpm && rm -f *.spec && sh precheckin.sh) after the Git checkout and that generates one or more .spec files
    • The rpm/precheckin.sh script MUST NOT assume that any .spec file exists, meaning that one cannot generate .spec file variants out of a base .spec file - the base .spec file must always be a template (e.g. .spec.in) -- this is to allow rm -f rpm/*.spec to remove leftover files, and to avoid situations like with bar.spec generated from foo.spec as described above
    • The rpm/precheckin.sh script SHOULD NOT have the executable bit (+x) set and the shebang line SHOULD BE #!/bin/sh (but is ignored, as it's run with sh precheckin.sh as above)
    • The rpm/precheckin.sh script MUST NOT use any bash-isms, only a normal Bourne shell should be assumed
    • Transitional feature: If the generated .spec file is stored in Git (as for all projects using generated .spec files up to now), it WILL BE overwritten by rpm/precheckin.sh
    • If rpm/precheckin.sh exists, it is always safe to run cd rpm && rm -f *.spec && sh precheckin.sh, this way, the script doesn't have to take care of removing outdated files (e.g. when a variant for an outdated device is removed, thereby causing less .spec files to be generated)
    • It is RECOMMENDED that projects that use rpm/precheckin.sh have rpm/*.spec (like this, with the glob) in their .gitignore files, to avoid accidentally adding/checking in generated .spec files during development

This specification allows local build tools such as mb2 to be eventually extended to generate and update .spec files on demand, and also provides a consistent way for developers to generate/update the .spec files manually (cd rpm && sh precheckin.sh).

In short:

  • If rpm/precheckin.sh exists, any tooling or developer should assume that it/he/she needs to first rm rpm/*.spec and then run sh precheckin.sh (inside the rpm/ directory as working directory) before the list of .spec files is up to date and before any package building can take place. Any change in the source directory - even outside of rpm/ - should cause a re-run of rpm/precheckin.sh, as the precheckin.sh script might parse source code files to obtain values used in the .spec file -- tar_git will run this script after checkout

Or in code (assume fail is a function that prints the error message and then exits with non-zero exit status):

# Right after checkout (tar_git) OR just before build (mb2 / manual build)
if [ -f rpm/precheckin.sh ]; then
    cd rpm
    rm -f *.spec
    sh precheckin.sh || fail "Could not generate .spec files with precheckin.sh"
    cd ..
fi

Advantages that this gives us:

  • We don't store redundant/generated files in Git
  • Generated files will always be up to date
  • We avoid users editing generated files and forgetting to update the templates
  • There is a single, documented and simple way of generating .spec files
  • Less frustration, more automation, consistency and fun(!)

Possible transitional disadvantages (= yes, we can't have nice things, there's a transitional cost):

  • Some existing precheckin.sh scripts might assume that they are run from the project root
  • Some existing precheckin.sh might require parameters, these need to be fixed
  • We need to educate developers how to use the new feature, and fix existing packaging
@iamer
Copy link
Contributor

iamer commented Feb 17, 2015

I like this idea better than the old spectacle yaml stuff. Also a lot of Mer packages are already using this precheckin.sh approach.

@thp
Copy link
Contributor Author

thp commented Mar 10, 2015

Any news on whether we can get this supported in tar_git? // cc @iamer @lbt @stskeeps

larstiq pushed a commit to larstiq/boss-launcher-webhook that referenced this issue Oct 8, 2015
Follows the specification as laid out by Thomas Perl in
MeeGoIntegration#5
@larstiq
Copy link
Contributor

larstiq commented Oct 12, 2015

As discussed with @iamer, "precheckin.sh" no longer describes what is going on. Renaming it to something (descriptive) that hasn't been in use before also allows us to sidestep any transitional headaches.

In summary, the procedure in #14 is now: cd rpm && rm -f *.spec && sh genspecs.sh

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

No branches or pull requests

3 participants