search

Strauman / travis-latexbuild

Building LaTeX packages using Travis-CI
12 stars 4 forks source link

A little large review #1

Closed PHPirates closed 5 years ago

PHPirates commented 5 years ago

I like the idea of using a docker image to build, I think it can make the configuration cleaner (just a few lines in .travis.yml and a package list I guess).

But I have a few questions:

Thanks again for working this out :) (I should really learn to work with Docker though)

Tasks

Strauman commented 5 years ago

Hey! I just decided to make this myself, and then share my work. I'm happy to generalize and improve on it so that it's easier and better for others to use, so thank you for this review 🥇!!

Again - thanks for the feedback and I'd love to hear from you again!

PHPirates commented 5 years ago

I just decided to make this myself, and then share my work.

Sounds like what I do in all my spare time :) and sometimes other people actually use things, which is fun.

All in all, I think this sounds good enough to make usable for a larger public :) It would be my preferred way when I have to build with pdflatex (I sometimes do since tectonic doesn't do -shell-escape).

Strauman commented 5 years ago

I did try the tectonic-docker version, but I think the issue was that packages weren't really updated or something? I remember having trouble building stuff.

Since I'm using latexmk to do the actual build, the plan is to add support for LuaTeX and XeTeX also!

Strauman commented 5 years ago

@PHPirates So I've made a temporary plan for this repo. You can head on over to the develop branch to see it (I've made a tentative README there for how I envision configuration). Please let me know what you think!

PHPirates commented 5 years ago

The thing is I already use the releases for actual releases, and that could be confusing for some.

Valid point, hadn't thought of that. I still think branches is not a good idea, but I don't know what else. So let's just leave it with branches, it's not that important.

Tectonic is limited, clearly it's not for everyone. About old packages: he was working on it yesterday https://tectonic.newton.cx/t/testers-wanted-candidate-new-texlive-2018-1-default-bundle/151

latexmk is cooler than I thought btw, maybe I should add a note for using latexmk instead of pdflatex in my readme.

PHPirates commented 5 years ago

The config.ini is very cool! I like the idea: people now need a very short .travis.yml and a very short config.ini if they want configuration options. All very clean and easy. [edit] and the file with the package list, of course

Strauman commented 5 years ago

The file with the package list will be gone! It'll all be in the tex-config.ini! I might need some help doing this, so if you're up for it, please let me know (I'd make you a collaborator).

PHPirates commented 5 years ago

That's true, makes sense. Yeah I'm definitely interested, I'll let you know if I have something. But I don't have much time, so it may be that you have already finished before I started :) we'll see. I just received another PR for my repo, this whole 'building-tex-on-travis' thing seems to have taken off without me doing anything besides reviewing other people's suggestions!

Strauman commented 5 years ago

Cool! It might be that the other repos are gonna cast a shadow over mine though, but I think this approach can be done more logical than the others? What do you think? [edit]: did you retry the build and get a new build time? [edit 2]: I see you now got 7 minutes, which is better. All of it is almost spent downloading. I'll add a medium size too! I have a few ideas to get it down even further also!

PHPirates commented 5 years ago

It might be that the other repos are gonna cast a shadow over mine though

Why do you think I'm collecting stuff in a central repo? So information from other repos doesn't get lost! And so that people don't get lost in a miriad of repos. :smile: If this works I'll be sure to add it to my readme, with a link to here.

Strauman commented 5 years ago

@PHPirates That'd be awesome, and definitely helps with my motivation!

Strauman commented 5 years ago

@PHPirates the config-style is now prototyped/alpha, so would you mind testing it?

PHPirates commented 5 years ago

Great, I hope I'll have time for that soon! I didn't get further yet than learning a bit about docker.

Strauman commented 5 years ago

Hey again! You could check out gh:strauman/latex-docker and the accompanying docker:strauman/tex. (I just took the tex-part of the docker out of this repo out and made it separate). Now to your remarks

Edit: @PHPirates Also, what do you think about dropping the config.ini and rather do all the config inside .travis.yml?

PHPirates commented 5 years ago

Oh that would certainly be preferable, but I don't really see how it would work. Using environment variables? (I'm starting to try things out now, assuming that things are pushed to the docker hub. I hope that makes sense. Apparently you push to docker hub manually, as it says 'pushed 2 minutes ago' right now but the latest commit is 2 hours ago?)

PHPirates commented 5 years ago

Things I found:

Is this what you want to try: use env: VAR=VAL environment variables in the travis yml, and then hope they are accessible from within the docker image?

The build with a MWE tex file succeeded (32 sec) with the current docker image, The build with all the packages also has succeeded now, and it says 3 min 53 sec! I don't know how that is possible though, or what changed.

Strauman commented 5 years ago

The full docker image takes a whooping 35 minutes to build, the medium about 15 mins, and the small a couple of minutes. The docker for this repo is built manually, but the one I made today containing only the TeX-part of the docker is built automatically on every push.

Also, I'm still trying to come up with a smart way to cache downloaded packages. One way would actually be to push a cached file back to the git repo somehow.

Strauman commented 5 years ago

Is this what you want to try: use env: VAR=VAL environment variables in the travis yml, and then hope they are accessible from within the docker image?

It's not what I'm trying to do, but that is very possible: docker run --env VAR="$VAR" ... which would accomplish that.

I'm thinking about doing the parsing of the .ini file from .travis.yml.

The build with a MWE tex file succeeded (32 sec) with the current docker image, The build with all the packages also has succeeded now, and it says 3 min 53 sec! I don't know how that is possible though, or what changed.

Yes. What took so long before was all the packages you wanted to install. Edit: No wait what?! I just built a fork of your repo: #3 and it seems to work great?!

~@PHPirates EDIT: Remember to switch out the travis-texbuild.sh (I can see here that you didn't)~

Strauman commented 5 years ago

@PHPirates I'm setting it up for you now.

PHPirates commented 5 years ago

Oh eh sorry I think I got you confused :) I first pushed that commit and then realised that you hadn't pushed the new docker image yet so I canceled the build and I meant to try later if it was really fixed. I did now, and it works! That's great. Thanks for taking all the time to verify it, you were quick to fix it :)

Strauman commented 5 years ago

It might take longer for me to fix it in the future due to starting on my master thesis soon. I'll keep an eye out though. It'd be awesome if you help. To motivate you I think you can combine some helping with the docker stuff:

I've made a new label for you called docker-related so that you might have overlap. For now it's just #11 (entailing #10 and #12) that is relevant.

PHPirates commented 5 years ago

Wait I don't get it exactly, I mean for people who want to use the push-to-branch functionality they will be willing to have the extra travis-texbuild.sh file in their repo right? I meant that just for people who don't use the push-to-branch functionality, they should be able to use your docker image without putting this file in their repos. Is that also what you meant? #12 seems to be what I meant though, but isn't that like already solved, and exactly what I did to test your latest fixes? My .travis.yml file is now:

sudo:       required
services:   docker

script:
- docker pull strauman/travis-latexbuild:small # small texlive scheme
- docker run --mount src=$TRAVIS_BUILD_DIR/,target=/repo,type=bind strauman/travis-latexbuild:small # also here

^ isn't this the solution to #12?

That's cool, at least you'll have something like a nice big tex file to build :smile: I'll definitely be fixing the things I put in my first post. I'll not be working on it every day but I'll keep it going.

Strauman commented 5 years ago

Your .travis.yml is outdated. See the one I suggested in PHPirates/travis-ci-latex-pdf#14, which would do this.

The approach you suggest is fine, but doesn't take the tex-scheme or pushtype into consideration (the latter I guess in unnecessary in light of #10).

A question for you: What OS do you use?

PHPirates commented 5 years ago

Ah I now see you changed it in that PR, hadn't noticed since the PR was to my master so it listed my changes as well.

Yes, I know that. So in my view there's two version of this: simple version (without tex-scheme, pushtype and without the extra shell file) and the customized version (with all that). I would let people choose between that, personally I wouldn't want the extra shell file but you probably will. So I'd write down both options. What do you think?

I use either windows 8.1 on my laptop or windows 7 or ubuntu 16.04 on my desktop, depending on what I want to do. Planning to install ubuntu 18.04 on my laptop as well though, the virtual machine is not everything.

Strauman commented 5 years ago

I put the PR to your master?! Sorry! I did intend to base it of your Strauman branch - sorry about that.

I agree, this was smart!

I was gonna write out how the shell scripts work for building and running this repos dockers locally, but I doubt it'll work on windows...

Strauman commented 5 years ago

@PHPirates heads up: you have to use comma to separate packages after the next push!

Strauman commented 5 years ago

Also, just for the record, since these are "nightly builds", I'm not practising a very good git flow, heh. But this will change once things get more stable

Strauman commented 5 years ago

Check out the quickstart branch!

PHPirates commented 5 years ago

I'm not like even trying to do something with Docker on Windows ;)

Check out the quickstart branch!

That's so cool! I like instructions which are basically "copy this and run". https://xkcd.com/1742/ I mean, I can remember when I first tried to build LaTeX on Travis. Well. That's history.

I'm going to try a few things now, linux is running :)

Strauman commented 5 years ago

https://xkcd.com/1742/

So spot on!

I'm going to try a few things now, linux is running

How to test execute_tests.sh without building the docker every time:

  1. Run build-docker-local.sh (builds the docker. Likely only necessary once)
  2. Run testrun-interactive.sh (runs the docker)
  3. Write tst in the docker to run the script as it would be run on travis.

If you now make changes to execute_tests.sh it is automatically updated inside the docker, and will run the updated version of it the next time you run the tst-command inside docker.

PHPirates commented 5 years ago

Thanks for the instructions! I had to look twice to see where the tst was coming from, but I get it now.

I ran a build again, but I'm not sure what went wrong: https://travis-ci.org/PHPirates/travis-ci-latex-pdf/builds/433140812 I can look at it later.

Strauman commented 5 years ago

Does the travis build you mentioned build locally? Can't really find the problem 😞

PHPirates commented 5 years ago

I can reproduce the problem on my local machine, but I still can't see the actual error. I am trying to debug it, but every time I run the docker image it has to download packages for like 15mins which is eating all my data and disk space. I tried to run it interactively but it just exited the docker run command after failing so that didn't help. I'm out of time now, but before I can do anything more I need to free disk space (edit: that was easy, I just deleted all docker containers and images)

Strauman commented 5 years ago

You shouldn't have to pull the docker every time! If you get the interactive to work, you install the packages you need once, and then compile the existing container as an image: https://docs.docker.com/engine/reference/commandline/commit/

Strauman commented 5 years ago

@PHPirates You don't have to build it every time! Just keep it open. When you edit the execute_tests then it's updated inside the docker. So you can just run tst again, which shouldn't reinstall the packages?

PHPirates commented 5 years ago

So is this what I should try to do: run a docker container from the image interactively such that it doesn't do anything yet (by editing the execute_tests as you say), then download the packages, then try running the tex file and find out what the problem is? And the next time I start up my computer I should be able to restart the docker container? You say I could compile the container with packages to an image, is then the advantage that I can start up more containers which would include the packages? I'm not sure why I would want that though.

I will fiddle with this sometime soon hopefully, thanks for the patience :)

Strauman commented 5 years ago

So is this what I should try to do: run a docker container from the image interactively such that it doesn't do anything yet (by editing the execute_tests as you say), then download the packages, then try running the tex file and find out what the problem is?

Yes - this makes the waiting for package downloads much shorter (since you only do it once). Thus you don't have to pull and re-build everything every time you change the contents of execute_tests. The workflow will be to do what you say once, then edit execute_tests and then run tst again.

And the next time I start up my computer I should be able to restart the docker container?

Not automatically. I'm just stating that it's possible if you "recompile" the container after you installed the packages.

You say I could compile the container with packages to an image, is then the advantage that I can start up more containers which would include the packages?

I think the main advantage is that you'd save time and not reinstalling the package every time you start the container. I don't know if you'll save significant amount of time, but if you have a lot of packages, you might.

I will fiddle with this sometime soon hopefully, thanks for the patience :)

I just appreciate your interest and help. I don't have too much time these days to work on it myself, but I'll make sure to keep monitoring the issue tracker and review any pull request. I also believe in "credit where credit is due" and if you do a significant amount of work, I'll make sure to mention you where ever it's deemed appropriate.