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

Introduce makefile #54

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

cryptointerest
Copy link

Indroduction of the makefile now with clean rebase.
I suggest postponing changing the scripts for adding the iso date formate to later until all changes have been merged.

These changes introduce the first batch of make automations for building the user guide as well as creating release tar balls.

New rules (one, don't pick up the phone, two ...):

Makefile

The Makefile supports the following rules.

Rules intended for regular use

  • template: Build the moderncv template template.tex with LuaLaTeX. This rule can be called in one of two ways:

    • make template: Build the template in casual style.
    • make template STYLE=<style>: Build the template in the style specified by
      <style>. <style> can be classic, casual, banking, oldstyle or fancy.
  • templates: Build the template template.tex with LuaLaTeX for all moderncv styles and move resulting pdf files to the folder examples/.

  • userguide: Build the user manual manual/moderncv_userguide.tex with LuaLaTeX. This rule calls the rule templates before compiling the documentation.

  • clean: Clean the clutter created by compiling of the documents.

  • delete:Delete template.pdf and manual/moderncv_userguide.pdf.

  • deleteexamples: Delete examples/ folder and remaining template example pdf files in folder manual/.

  • force: Force rebuilding the user guide by running the rules delete deleteexamples userguide and clean `clean

Rules intended for package maintainers

  • version: Update the version information (version number and date) of all moderncv files (*.sty, moderncv.cls, *.tex). This rule can be called in two different ways. Note, however, that it is intended to be called by the rule release and usually does not need to be called explicitly.
    • make version: Called in this way the version number is obtained through git describe --tags. If this information is newer all moderncv files get updated.
    • make version NEW=<version number>: Optionally, the desired version number <version number> can be specified.
  • tarball: Create a new release tarball suitable for upload to CTAN. If the example/ folder is present, it gets included in the tar archive. Similary, all pdf files in the manual/ folder get included aswell. This rule is intended to be called by the rule release and usually does not need to be called explicitly.
  • release:Update the version information, rebuild examples as well as the user guide and create a releasable tarball including everything. In this way the tarball on CTAN contains ready made pdf files.

Copy link
Member

@stephanlachnit stephanlachnit left a comment

Choose a reason for hiding this comment

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

Thanks for the work, please take a look at my comments.

.gitignore Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
Comment on lines +122 to +142
tarball:
# build a tarball for release puposes. If the examples directory exist,
# include them
# remove existing tarballs
rm -f *.gz *.tar
# create tar with all files in git repo
git archive HEAD > $(TARBALL)
# remove git specific files
tar -f $(TARBALL) --delete .github/ .gitignore create-release-tarball.sh
# if examples exist include them in the tarball
if [[ -d "$(EXAMPLESDIR)" ]]; then
tar -rf $(TARBALL) --append $(EXAMPLESDIR)
fi
# include precompiled template pdfs and userguide from manual folder,
# the idea being that the userguide can be build from the manual folder
# and has everything that it needs to compile. If the release method gets
# called this ensures that a precompiled version of the userguide is
# included in the tar ball.
tar -rf $(TARBALL) --append $(MANUALDIR)
# compress
gzip $(TARBALL)
Copy link
Member

Choose a reason for hiding this comment

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

Ok first thing, with this you want to remove create-release-tarball.sh from the repo, as it is essentially the same. Second, the reason for the tarball is the upload to CTAN, so the example creating should be consistent. I personally don't think they should be included in the release tarball, only the manual.

The part where you add the manual should also be in the commit where the actual tarball will be added.

Copy link
Author

Choose a reason for hiding this comment

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

I thought it might be a good thing to have examples precompiled for users to have a look at. Therefore I included them.
To not clutter the repo with precompiled pdfs, I only included them in the release tar ball, so that they are also available on CTAN for download immediatelly.

Copy link
Member

Choose a reason for hiding this comment

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

Yes I agree that precompiled pdf shouldn't be in the repo. I checked a couple of other packages, and I guess it's fine if we add one example to the release tarball which can be linked to at the CTAN page (I wouldn't add all of them, I think that's a bit useless).

In any case, it's important that this is consistent, and not depending on the existence of a folder.

Comment on lines +10 to +17
# user callable NEW option, to specify the new version. If unspecified, the
# new version gets determined by git.
ifdef NEW
VERSIONNEXT = $(NEW)
else
VERSIONNEXT = $(shell git describe --tags --dirty)
endif
VERSIONDATENEXT = $(shell date +"%Y\/%m\/%d")
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 something I will comment later on, but I think this can be solved more elegantly.

Copy link
Member

Choose a reason for hiding this comment

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

See below, please remove this.

Comment on lines +80 to +81
lualatex $(MANUALTEX)
lualatex $(MANUALTEX)
Copy link
Member

Choose a reason for hiding this comment

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

Why not latexmk?

Copy link
Author

Choose a reason for hiding this comment

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

I have had issues in the past with latexmk, therefor I avoid it.

Copy link
Member

Choose a reason for hiding this comment

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

Can we go with latexmk? I think overall it is the better choice, since it does the rerunning automatically until all references are cleared. Maybe it's not perfect in every edge case, but here it should be fine.

We can also enforce lualatex: latexmk -pdflua $(MANUALTEX)

Comment on lines +86 to +89
# Upate version information and date of all moderncv files. A call make version
# will define VERSIONNEXT=$(shell git describe --tags). Alternatively, call
# "make version NEW=v5.13.298" to set v5.13.298 as the new version number.
# The date gets calculated by the date function.
Copy link
Member

Choose a reason for hiding this comment

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

Ok so this just doesn't work in this order. We first need to update the files, and then tag it via git. The other way round is completely useless (sorry). The version should be set by the user, and then git tagged.

Copy link
Author

Choose a reason for hiding this comment

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

No. This rule automates updating all files, that's the point. The makefile provides this method and the VERSION variable to make it easy to update all files in one go. From the explanation of the README:

version: Update the version information (version number and date) of all moderncv files (*.sty, moderncv.cls, *.tex). This rule can be called in two different ways. Note, however, that it is intended to be called by the rule release and usually does not need to be called explicitly.

  • make version: Called in this way the version number is obtained through git describe --tags. If this information is newer all moderncv files get updated.
  • make version NEW=<version number>: Optionally, the desired version number <version number> can be specified.

I first intended this rule to be called in the second way by the maintainer to first update all files as you said.
Then I thought it might be usefull to automate this with git, so that as a mainteiner you only need to run one command at the end and all information is up to date automatically.

Copy link
Author

Choose a reason for hiding this comment

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

I was intending to reduce your work load for releasing tarball to CTAN ;-)

Copy link
Member

Choose a reason for hiding this comment

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

make version: Called in this way the version number is obtained through git describe --tags. If this information is newer all moderncv files get updated.

Yes exactly that one doesn't make sense, since this will always be outdated. First the change of version number, then the git tag.

Comment on lines +151 to +153
rm -fv *.acn *.acr *.alg *.aux *.bcf *.blg *.dvi *.fdb_latexmk *.fls;
rm -fv *.glg *.idx *.ilg *.ist *.spl *.lof *.log *.lot *.out *.pdfsync;
rm -fv *.run.xml *.snm *.synctex.gz *.tdo *.toc *.vrb *blx.bib *~;
Copy link
Member

Choose a reason for hiding this comment

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

Why not latexmk -C?

Copy link
Author

Choose a reason for hiding this comment

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

as I said, I have had problem with latexmk

Copy link
Member

Choose a reason for hiding this comment

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

Please go with latexmk. It doesn't remove all the files, but to does this code (e.g. .bbl files are also generated when using biber).

manual/format_files_for_userguide.py Show resolved Hide resolved
@stephanlachnit
Copy link
Member

stephanlachnit commented Apr 30, 2021

Btw can you update the CI workflow to build all?

Copy link
Member

@stephanlachnit stephanlachnit left a comment

Choose a reason for hiding this comment

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

Alright, I think I have now commented on everything that needs changing.

```
in a terminal. After completion of the compilation precompiled versions of the template in all styles can be found in the folder `examples` and
the user guide in the folder `manual`.
Alternatively get the tar ball from [CTAN](https://ctan.org/pkg/moderncv?lang=de). The examples as well as the documentation are already prebuilt in that tarball.
Copy link
Member

Choose a reason for hiding this comment

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

Url is in german, change it to https://ctan.org/pkg/moderncv

Comment on lines +80 to +81
lualatex $(MANUALTEX)
lualatex $(MANUALTEX)
Copy link
Member

Choose a reason for hiding this comment

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

Can we go with latexmk? I think overall it is the better choice, since it does the rerunning automatically until all references are cleared. Maybe it's not perfect in every edge case, but here it should be fine.

We can also enforce lualatex: latexmk -pdflua $(MANUALTEX)

Comment on lines +151 to +153
rm -fv *.acn *.acr *.alg *.aux *.bcf *.blg *.dvi *.fdb_latexmk *.fls;
rm -fv *.glg *.idx *.ilg *.ist *.spl *.lof *.log *.lot *.out *.pdfsync;
rm -fv *.run.xml *.snm *.synctex.gz *.tdo *.toc *.vrb *blx.bib *~;
Copy link
Member

Choose a reason for hiding this comment

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

Please go with latexmk. It doesn't remove all the files, but to does this code (e.g. .bbl files are also generated when using biber).

Comment on lines +159 to +165
delete:
rm -fv $(TEMPLATEPDF)
rm -fv $(MANUALPDF)

deleteexamples:
rm -rfv $(EXAMPLESDIR)
rm -fv $(MANUALDIR)/*.pdf
Copy link
Member

Choose a reason for hiding this comment

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

I guess we can merge these simply to delete, that should be enough. Also, delete should call clean.

sed -i $$sedstring $(TEMPLATE)


userguide: templates $(MANUAL)
Copy link
Member

Choose a reason for hiding this comment

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

It's called manual everywhere else, so let's change this to usermanual.

Comment on lines +94 to +102
# we need to split the $(VERSION) date format into substrings to using
# sed. This is due to / being a special character in sed.
YEAR=$(shell echo $(VERSIONDATE) | cut -d "/" -f 1)
MONTH=$(shell echo $(VERSIONDATE) | cut -d "/" -f 2)
DAY=$(shell echo $(VERSIONDATE) | cut -d "/" -f 3)
# update version info and date of *.sty, *.cls and *.tex files
# prepare find and replace with sed
findstr="$$YEAR\\/$$MONTH\\/$$DAY $(VERSION)"
replacestr="$(VERSIONDATENEXT) $(VERSIONNEXT)"
Copy link
Member

Choose a reason for hiding this comment

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

Ok I see now why you want to have the date in the makefile, but I still think this is not a particular good design. You can use this regex \d{4}\-\d{2}\-\d{2}\sv\d+.\d+.\d+ (via sed -E) to find the dates, and replace it with $(date -u -I) $(NEWVERSION)

Before, check if NEWVERSION is set, if not, then abort.

Comment on lines +114 to +119
# update VERSION and VERSIONDATE variable of this very file
sed -i "s/VERSION = $(VERSION)/VERSION = $(VERSIONNEXT)/g" $(MODERNCVDIR)/Makefile
findstr="VERSIONDATE = $$YEAR\\/$$MONTH\\/$$DAY"
replacestr="VERSIONDATE = $(shell date +"%Y")\\/$(shell date +"%m")\\/$(shell date +"%d")"
sed -i "s/$$findstr/$$replacestr/g" $(MODERNCVDIR)/Makefile
unset findstr; unset replacestr
Copy link
Member

Choose a reason for hiding this comment

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

With the comment mentioned above, it can be removed.

rm -rfv $(EXAMPLESDIR)
rm -fv $(MANUALDIR)/*.pdf

force: delete deleteexamples userguide clean
Copy link
Member

Choose a reason for hiding this comment

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

I think just force is a bit ambiguous. Let's go with forcerebuild instead.

# compress
gzip $(TARBALL)

release: userguide version clean tarball
Copy link
Member

Choose a reason for hiding this comment

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

Here we would do delete (clean rebuild), version, then force (so we have the new version in the manual), then tarball and then do git commit -a -s -C $(NEWVERSION), then git tag -s HEAD~1 (I'll need to double if that git commit runs like that without prompting an editor, but that's the idea at least).

Comment on lines +10 to +17
# user callable NEW option, to specify the new version. If unspecified, the
# new version gets determined by git.
ifdef NEW
VERSIONNEXT = $(NEW)
else
VERSIONNEXT = $(shell git describe --tags --dirty)
endif
VERSIONDATENEXT = $(shell date +"%Y\/%m\/%d")
Copy link
Member

Choose a reason for hiding this comment

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

See below, please remove this.

@cryptointerest
Copy link
Author

Thanks for all the comments. I will do as fast as I can, but I have some paper submission stuff etc. going on, so I'll have to see when I can work on it.

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

Successfully merging this pull request may close these issues.

None yet

2 participants