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

Rebuild intermediate dependencies [broken] #17

Closed
wants to merge 1 commit into from

Conversation

btrask
Copy link
Contributor

@btrask btrask commented Mar 22, 2015

Several tests are failing after rebuilding scanners.c. This might be a problem with the version of re2c I have, but I'm not sure.

This uncovers a problem with scanners.re that was obscured because scanners.c was obsolete.
@btrask btrask mentioned this pull request Mar 22, 2015
@jgm
Copy link
Member

jgm commented Mar 22, 2015

What version of re2c are you using, on what platform?
We've had some problems with some versions.

+++ Ben Trask [Mar 22 15 15:07 ]:

Several tests are failing after rebuilding scanners.c. This might be a problem with the version of re2c I have, but I'm not sure.
You can view, comment on, or merge this pull request online at:

#17

-- Commit Summary --

  • Rebuild intermediate dependencies.

-- File Changes --

M Makefile (2)
M src/scanners.c (1870)

-- Patch Links --

https://github.com/jgm/cmark/pull/17.patch
https://github.com/jgm/cmark/pull/17.diff


Reply to this email directly or view it on GitHub:
#17

@btrask
Copy link
Contributor Author

btrask commented Mar 22, 2015

re2c 0.14.1 on Linux. Downloaded from here. I'll try with 0.13.6 which seems to be the version you're using.

@nwellnhof
Copy link
Contributor

The test failures are the same that I got with re2c 0.13.7.5. See commonmark/commonmark-spec#199

@nwellnhof
Copy link
Contributor

Also, we're not rebuilding these files for a reason. It simplifies the build on platforms like Windows and there are problems with some versions of re2c and gperf. What is the goal of the pull request?

@btrask
Copy link
Contributor Author

btrask commented Mar 24, 2015

The goal is to keep the files in sync in case the .re version is changed (like in my other patch #18). When I rebuilt .c and started getting test failures, I first thought it was because it was out of date or changed manually.

I don't know the situation on Windows but I'd suggest including re2c in the cmark repo so that it can be relied on.

I'd also submit a PR to add more information recommending re2c v0.13.6 to the readme, if that would be welcome.

@nwellnhof
Copy link
Contributor

To add a warning about buggy re2c versions would certainly be nice. Ideally, we'd check the version before running re2c. If I find the time, I'll try to create a minimal test case and create a ticket in the re2c bug tracker.

Bundling re2c or gperf is a possibility but has some downsides:

  • It's a lot of effort to integrate seamlessly into the build process.
  • It slows down the build and increases the size of the tar ball.

For the time being, I'd keep these generated files in version control and update them manually. Maybe we could add some prominent notices that this is necessary.

@btrask
Copy link
Contributor Author

btrask commented Mar 24, 2015

Even if re2c isn't bundled, by having the make rules set up, you'll get a build failure if you touch the source files. This seems like desirable behavior to me, it's basically the warning you suggest. You can use git reset to undo the changes if you don't have re2c installed.

@nwellnhof
Copy link
Contributor

Using a version control system like Git makes the time stamps of files unpredictable. After an initial checkout with git clone or after switching branches, scanners.re might be newer than scanners.c resulting in an attempt to rebuild. The behavior is random. After changing and resetting, scanners.re will always have a later timestamp. Git doesn't track modification time.

@jgm
Copy link
Member

jgm commented Mar 25, 2015

+++ Nick Wellnhofer [Mar 24 15 08:35 ]:

Bundling re2c or gperf is a possibility but has some downsides:

  • It's a lot of effort to integrate seamlessly into the build process.
  • It slows down the build and increases the size of the tar ball.

For the time being, I'd keep these generated files in version control and update them manually. Maybe we could add some prominent notices that this is necessary.

I agree. It's also nice that people can build from git checkout without installing anything other than a C compiler.

@btrask
Copy link
Contributor Author

btrask commented Mar 26, 2015

I see that I was wrong about modification times in Git. Thanks for the correction.

Withdrawing this proposal.

@btrask btrask closed this Mar 26, 2015
PKRoma pushed a commit to PKRoma/cmark that referenced this pull request Aug 19, 2018
mbernson pushed a commit to mbernson/cmark that referenced this pull request Jul 6, 2020
…uoted-includes

Use quoted includes for cmark_export.h and cmark_version.h
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.

3 participants