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

tectonic 0.1.6 (new formula) #14575

Closed
wants to merge 1 commit into from
Closed

tectonic 0.1.6 (new formula) #14575

wants to merge 1 commit into from

Conversation

iKevinY
Copy link
Contributor

@iKevinY iKevinY commented Jun 14, 2017

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

Thanks to #14490 getting merged in, we're closer to a functional formula for Tectonic! I'm submitting this as a WIP to move Homebrew-specific discussion out of tectonic-typesetting/tectonic#4, but there's a couple of things that need still need to be addressed:

  • Ideally, the test block should invoke tectonic. Unfortunately, a bunch of rather large TeX packages need to be downloaded in order to process even a minimal (empty) document.
  • brew audit fails because the harfbuzz dependency needs to be installed with-graphite2.

@ilovezfs ilovezfs added new formula PR adds a new formula to Homebrew/homebrew-core rust Rust use is a significant feature of the PR or issue labels Jun 14, 2017
@ilovezfs
Copy link
Contributor

@iKevinY this will take care of harfbuzz: #14583

@ilovezfs
Copy link
Contributor

@iKevinY regarding the audit

Dependency 'icu4c' may be unnecessary as it is provided by macOS; try to build this formula without it.

Have you tested a build that doesn't depend on the icu4c formula?

@ilovezfs ilovezfs added the build failure CI fails while building the software label Jun 14, 2017
@ilovezfs
Copy link
Contributor

@iKevinY the build fails on Yosemite, and also fails on El Capitan unless I have Xcode not just the CLT.

@ilovezfs
Copy link
Contributor

@iKevinY I tried it with paper.tex from the repo, and it seems to error out: https://gist.github.com/ilovezfs/befd84e087fad9c72422e90b09418b70

iMac-TMP:footec joe$ tectonic paper.tex 
note: this is a BETA release; report issues at https://github.com/tectonic-typesetting/tectonic/issues
Running TeX ...
error: something bad happened inside TeX; its output follows:

===============================================================================
(paper.tex
LaTeX2e <2016/03/31>
Babel <3.9r> and hyphenation patterns for 83 language(s) loaded.
(aastex6.cls

LaTeX Warning: You have requested document class `aastex6',
               but the document class provides `aastex'.

Document Class: aastex 2016/02/16 1.0/AAS markup document class
(times.sty) (lineno.sty) (revtex4-1.cls
Document Class: revtex4-1 2010/07/25/20:33:00 4.1r (http://publish.aps.org/revt
ex4/ for documentation)
ltxutil[2010/07/25/20:33:00 4.1r utilities package (portions licensed from W. E
. Baxter web at superscript.com)]
ltxfront[2010/07/25/20:33:00 4.1r frontmatter package (AO,DPC)]
ltxgrid[2010/07/25/20:33:00 4.1r page grid package (portions licensed from W. E
. Baxter web at superscript.com)]

Class revtex4-1 Warning: Failed to recognize \@vspace, \@vspacer, \@no@pgbk, \@
newline, and \\; no patches applied. Please get a more up-to-date class, .

(aps4-1.rtx) (aps10pt4-1.rtx) (textcase.sty) (url.sty) (natbib.sty)
(revsymb4-1.sty)) (latexsym.sty) (graphicx.sty (keyval.sty) (graphics.sty
(trig.sty) (graphics.cfg) (xetex.def (infwarerr.sty) (ltxcmds.sty))))
(amssymb.sty (amsfonts.sty)) (epsf.sty
This is `epsf.tex' v2.7.4 <14 February 2011>
) (longtable.sty) (hyperref.sty (hobsub-hyperref.sty (hobsub-generic.sty))
(ifxetex.sty) (auxhook.sty) (kvoptions.sty) (pd1enc.def) (hyperref.cfg))

Package hyperref Message: Driver (autodetected): hxetex.

(hxetex.def (puenc.def) (stringenc.sty) (rerunfilecheck.sty)) (xcolor.sty
(color.cfg)) (array.sty) (ulem.sty))===============================================================================

error: failed to open input file "mode"

@dunn dunn added the needs response Needs a response from the issue/PR author label Jun 14, 2017
@iKevinY
Copy link
Contributor Author

iKevinY commented Jun 15, 2017

this will take care of harfbuzz: #14583

Great, thanks!

Have you tested a build that doesn't depend on the icu4c formula?

Even when removing the explicit icu4c dependency from the formula, it still gets downloaded as part of the installation process (presumably because harfbuzz depends on it).

Maybe @pkgw has some insights into the build failures and TeX errors.

@BrewTestBot BrewTestBot removed the needs response Needs a response from the issue/PR author label Jun 15, 2017
@ilovezfs
Copy link
Contributor

@iKevinY any updates here?

@JCount JCount added the needs response Needs a response from the issue/PR author label Jun 25, 2017
@iKevinY
Copy link
Contributor Author

iKevinY commented Jun 25, 2017

@ilovezfs I'll post on Tectonic's forum to see if anyone has suggestions on how to fix these issues.

@BrewTestBot BrewTestBot removed the needs response Needs a response from the issue/PR author label Jun 25, 2017
@alexreg
Copy link
Contributor

alexreg commented Jun 27, 2017

@ilovezfs Did you have any more luck with a minimal tex file?

@ilovezfs
Copy link
Contributor

@alexreg Correct. It did work with a minimal tex file when had I tried that, just not for the paper.tex in the tectonic repo.

the build fails on Yosemite, and also fails on El Capitan unless I have Xcode not just the CLT.

That is probably a higher priority problem than getting paper.tex to work :)

@pkgw
Copy link
Contributor

pkgw commented Jun 27, 2017

Sorry, I'm really slammed at work right now and haven't been able to spend any time on this. Unfortunately it looks like this will continue to be the case for the next few weeks at least.

@ilovezfs
Copy link
Contributor

@pkgw no worries! Thanks for the status update.

@alexreg
Copy link
Contributor

alexreg commented Jun 28, 2017

Just a note: the rust dependency should be optional, to allow for e.g. rustup versions of Rust.

depends_on "rust" => [:build, :recommended]

@pkgw
Copy link
Contributor

pkgw commented Jul 8, 2017

Heads up: I've just published version 0.1.6 of the Tectonic rust crate.

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 8, 2017

@pkgw neat! Note that https://github.com/tectonic-typesetting/tectonic/releases/latest still points to 0.1.5

@ilovezfs ilovezfs changed the title tectonic 0.1.5 (new formula) tectonic 0.1.6 (new formula) Jul 8, 2017
@pkgw
Copy link
Contributor

pkgw commented Jul 8, 2017

@ilovezfs Yeah — will update that after I write up a release announcement.

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 8, 2017

@iKevinY would be cool to get the Homebrew formula into the release notes :) Wonder if we should just depends_on :el_capitan it for now.

@pkgw
Copy link
Contributor

pkgw commented Jul 8, 2017

@iKevinY @ilovezfs There's a ton to write for the release notes and I need to revamp the website a bit too, so you probably have some time :-)

Oh, and I need to update my Conda packages, which involves rebuilding my packages of the Rust/Cargo toolchain ...

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 8, 2017

@pkgw I'm still seeing this for paper.tex

iMac-TMP:paper joe$ tectonic paper.tex 
note: this is a BETA release; ask questions and report bugs at https://tectonic.newton.cx/
note: indexing https://purl.org/net/pkgwpub/tectonic-default
note: resolved to https://dl.bintray.com/pkgw/tectonic/tl2016extras/2016.0r4/tlextras-2016.0r4.tar
note: downloading SHA256SUM
note: generating format "latex"
Running TeX ...
error: something bad happened inside TeX; its output follows:

===============================================================================
(paper.tex
LaTeX2e <2016/03/31>
Babel <3.9r> and hyphenation patterns for 83 language(s) loaded.
(aastex6.cls

LaTeX Warning: You have requested document class `aastex6',
               but the document class provides `aastex'.

Document Class: aastex 2016/02/16 1.0/AAS markup document class
(times.sty) (lineno.sty) (revtex4-1.cls
Document Class: revtex4-1 2010/07/25/20:33:00 4.1r (http://publish.aps.org/revt
ex4/ for documentation)
ltxutil[2010/07/25/20:33:00 4.1r utilities package (portions licensed from W. E
. Baxter web at superscript.com)]
ltxfront[2010/07/25/20:33:00 4.1r frontmatter package (AO,DPC)]
ltxgrid[2010/07/25/20:33:00 4.1r page grid package (portions licensed from W. E
. Baxter web at superscript.com)]

Class revtex4-1 Warning: Failed to recognize \@vspace, \@vspacer, \@no@pgbk, \@
newline, and \\; no patches applied. Please get a more up-to-date class, .

(aps4-1.rtx) (aps10pt4-1.rtx) (textcase.sty) (url.sty) (natbib.sty)
(revsymb4-1.sty)) (latexsym.sty) (graphicx.sty (keyval.sty) (graphics.sty
(trig.sty) (graphics.cfg) (xetex.def (infwarerr.sty) (ltxcmds.sty))))
(amssymb.sty (amsfonts.sty)) (epsf.sty
This is `epsf.tex' v2.7.4 <14 February 2011>
) (longtable.sty) (hyperref.sty (hobsub-hyperref.sty (hobsub-generic.sty))
(ifxetex.sty) (auxhook.sty) (kvoptions.sty) (pd1enc.def) (hyperref.cfg))

Package hyperref Message: Driver (autodetected): hxetex.

(hxetex.def (puenc.def) (stringenc.sty) (rerunfilecheck.sty)) (xcolor.sty
(color.cfg)) (array.sty) (ulem.sty))===============================================================================

error: failed to open input file "mode"

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 8, 2017

Is there a basic doc you like to test with it?

@pkgw
Copy link
Contributor

pkgw commented Jul 8, 2017

@ilovezfs That's the one that I use. That's a strange failure, since the input file mode.tex should just be sitting right in the tests/xenia directory.

@pkgw
Copy link
Contributor

pkgw commented Jul 8, 2017

  1. Hmm, looks like some C++ standard library versioning issue. Maybe something like needing to link with libc++ instead of libstdc++, that sort of thing? I'm not familiar with the details here but I bet some Googling will reveal people who have grappled with the same problem.

  2. This is a bit of cargo-cult on my part but as far as I can tell it's needed. See this lengthy GitHub issue.

  3. It does seem reasonable to avoid this. The Tectonic source tree includes all of the files needed to build the plain format, which is built and exercised in the standard test suite. If you're running the built-in tests with cargo test I don't think it's necessary to also run the xenia test.

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 8, 2017

You misunderstand. If it's a :recommended dependency, then it allows users who use Rustup rust to bypass installing a Homebrew Rust, which is advantageous.

rustup's rust won't be in the PATH. And it's not a concern because rust is a build-time only requirement anyway, so people who pour the pre-built bottle binary have nothing to worry about.

@pkgw
Copy link
Contributor

pkgw commented Jul 8, 2017

@alexreg Strange! There is definitely nothing in Tectonic that has anything to do with sqlite, so it must be something on the Homebrew side.

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 8, 2017

Error: /usr/local/opt/sqlite not present or broken

This is a problem with your brew installation. Probably just need to

rm /usr/local/opt/sqlite
rm -rf /usr/local/Cellar/sqlite
brew prune
brew install sqlite

@alexreg
Copy link
Contributor

alexreg commented Jul 8, 2017

Hmm, looks like some C++ standard library versioning issue. Maybe something like needing to link with libc++ instead of libstdc++, that sort of thing? I'm not familiar with the details here but I bet some Googling will reveal people who have grappled with the same problem.

Yes, looks like exactly that. C++11 vs. C++.

@alexreg
Copy link
Contributor

alexreg commented Jul 8, 2017

You misunderstand. If it's a :recommended dependency, then it allows users who use Rustup rust to bypass installing a Homebrew Rust, which is advantageous.
rustup's rust won't be in the PATH. And it's not a concern because rust is a build-time only requirement anyway, so people who pour the pre-built bottle binary have nothing to worry about.

It's still extra disk-space, which is non-ideal. But I see your point about PATH. (Well, it may be in the PATH, but not necessarily.) Ideally we'd have something like :rust, but I know the Homebrew maintainer is strongly against these things, for some odd reason, which I forget.

It's also annoying that Homebrew Rust conflicts with Rustup Rust...

Error: The `brew link` step did not complete successfully
The formula built, but is not symlinked into /usr/local
Could not symlink etc/bash_completion.d/cargo
Target /usr/local/etc/bash_completion.d/cargo
already exists. You may want to remove it:
  rm '/usr/local/etc/bash_completion.d/cargo'

To force the link and overwrite all conflicting files:
  brew link --overwrite rust

To list all files that would be deleted:
  brew link --overwrite --dry-run rust

Possible conflicting files are:
/usr/local/etc/bash_completion.d/cargo
/usr/local/share/zsh/site-functions/_cargo

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 8, 2017

If you're running the built-in tests with cargo test I don't think it's necessary to also run the xenia test.

@pkgw we cannot run cargo test in the test block because the build-time-only rust dependency will be unavailable at that point.

@pkgw
Copy link
Contributor

pkgw commented Jul 8, 2017

@ilovezfs Ah, I see. If small downloads are acceptable, you could test with a nonsense document that uses the plain format, which requires many fewer resources. If you really want to avoid any downloads at all, that's trickier ... the distribution could perhaps embed a tiny bundle file with the resources needed to create the plain format.

I'm going to be offline for a while starting now. I still need to write the release notes so the announcement is very unlikely to go out before tomorrow at this rate.

@alexreg
Copy link
Contributor

alexreg commented Jul 8, 2017

Just want to confirm: all builds fine for me now! (Please see above post still though.)

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 8, 2017

Just want to confirm: all builds fine for me now!

excellent!

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 8, 2017

I'm going to be offline for a while starting now. I still need to write the release notes so the announcement is very unlikely to go out before tomorrow at this rate.

hehe feel free to blame me :)

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 8, 2017

OK. The build failure is fixed by adding

    ENV.cxx11
    ENV["MACOSX_DEPLOYMENT_TARGET"] = MacOS.version

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 8, 2017

I am more than happy to review a PR for a lighter-weight, or even offline test, but I think this is good to ship now.

@iKevinY
Copy link
Contributor Author

iKevinY commented Jul 8, 2017

Oh wow, what a nice surprise to see when checking my GitHub notifications! Thank you to everyone for getting this formula working! 😄

@ilovezfs ilovezfs closed this in f44bd68 Jul 8, 2017
@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 8, 2017

Thanks for the 🆕 formula @iKevinY! Shipped!

Also, thanks @pkgw and @alexreg for the helping hands! It's much appreciated. :)

@alexreg
Copy link
Contributor

alexreg commented Jul 8, 2017 via email

@iKevinY iKevinY deleted the tectonic branch July 8, 2017 19:48
@pkgw
Copy link
Contributor

pkgw commented Jul 9, 2017

Great! I've updated the website to mention Homebrew as an option now. (Relevant commit.)

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 9, 2017

@pkgw awesome! Thanks :)

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 9, 2017

@pkgw note that I made --with-graphite2 the default for harfbuzz now for the sake of tectonic, so the source-build instructions on your site are slightly outdated.

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 9, 2017

See #14583

@pkgw
Copy link
Contributor

pkgw commented Jul 9, 2017

Oh, nice, thanks! Can I trouble you to submit a PR on the website repo to make the language more accurate?

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 9, 2017

@pkgw I'd be happy to.

@ilovezfs
Copy link
Contributor

ilovezfs commented Jul 9, 2017

@pkgw
Copy link
Contributor

pkgw commented Jul 12, 2017

@ilovezfs Re: the zlib stuff, tectonic #120 may have fixed this on our end, so you may be able to drop the zlib dependency when the next version comes out.

@ilovezfs
Copy link
Contributor

@pkgw yes it looks like it works to me:

iMac-TMP:~ joe$ mdutil -t `which tectonic`
/usr/local/Cellar/tectonic/HEAD-8201ce1/bin/tectonic
iMac-TMP:~ joe$ brew linkage tectonic
System libraries:
  /System/Library/Frameworks/AppKit.framework/Versions/C/AppKit
  /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation
  /System/Library/Frameworks/CoreGraphics.framework/Versions/A/CoreGraphics
  /System/Library/Frameworks/CoreText.framework/Versions/A/CoreText
  /System/Library/Frameworks/Foundation.framework/Versions/C/Foundation
  /System/Library/Frameworks/Security.framework/Versions/A/Security
  /usr/lib/libSystem.B.dylib
  /usr/lib/libc++.1.dylib
  /usr/lib/libobjc.A.dylib
  /usr/lib/libz.1.dylib
Homebrew libraries:
  /usr/local/opt/freetype/lib/libfreetype.6.dylib (freetype)
  /usr/local/opt/graphite2/lib/libgraphite2.3.dylib (graphite2)
  /usr/local/opt/harfbuzz/lib/libharfbuzz-icu.0.dylib (harfbuzz)
  /usr/local/opt/harfbuzz/lib/libharfbuzz.0.dylib (harfbuzz)
  /usr/local/opt/icu4c/lib/libicudata.58.2.dylib (icu4c)
  /usr/local/opt/icu4c/lib/libicuuc.58.dylib (icu4c)
  /usr/local/opt/libpng/lib/libpng16.16.dylib (libpng)
iMac-TMP:~ joe$ brew config | grep Xcode
Xcode: N/A
iMac-TMP:~ joe$ 

@Homebrew Homebrew locked and limited conversation to collaborators May 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
new formula PR adds a new formula to Homebrew/homebrew-core rust Rust use is a significant feature of the PR or issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants