Skip to content

Fix Pandoc version check in configure #22474

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

Merged
merged 1 commit into from
Mar 6, 2015
Merged

Fix Pandoc version check in configure #22474

merged 1 commit into from
Mar 6, 2015

Conversation

iKevinY
Copy link
Contributor

@iKevinY iKevinY commented Feb 18, 2015

Executing configure seems to create the following error due to how the script parses Pandoc's version:

./configure: line 705: [: pandoc: integer expression expected
./configure: line 705: [: 1.12.4.2: integer expression expected

This issue seems to stem from a discrepancy between BSD and GNU versions of sed. This patch changes the sed command to use an extended regex, which works with both flavours of sed.

@rust-highfive
Copy link
Contributor

r? @huonw

(rust_highfive has picked a reviewer for you, use r? to override)

@iKevinY
Copy link
Contributor Author

iKevinY commented Feb 18, 2015

After trying a few things, I decided upon this solution, which uses grep to isolate the MAJOR.MINOR component of the version number, and changed the shell globs to use . as the delimiting character.

@alexcrichton
Copy link
Member

@bors: r+ b720412a673e2d2afdd574841475d32f157fb593

@iKevinY
Copy link
Contributor Author

iKevinY commented Feb 24, 2015

@alexcrichton Do you think that using grep -Eo and head is portable enough to work on all systems? I don't have too much experience with anything outside of OS X, and I don't want this patch to fix behaviour on OS X just to have it break on other systems.

@alexcrichton
Copy link
Member

Hm, I'm not quite sure. I know that @brson has been doing a bunch of shell scripting recently though and may know better than I.

r? @brson

@rust-highfive rust-highfive assigned brson and unassigned huonw Feb 24, 2015
@alexcrichton
Copy link
Member

@bors: r-

@brson
Copy link
Contributor

brson commented Feb 24, 2015

I think it's ok. -E is common. -o I'm not sure of (doesn't work on Solaris).

Better than the existing breakage anyway.

@iKevinY
Copy link
Contributor Author

iKevinY commented Feb 24, 2015

Hmm, is it safe to use \d or should I just leave it as [0-9]? I did some really basic testing on an Ubuntu VPS and grep -E doesn't seem to like \d.

Edit: I've changed \d to [0-9] since it seems as though the \d character class isn't recognized by all versions of grep. This still leaves the issue of grep -o, though.

@iKevinY
Copy link
Contributor Author

iKevinY commented Feb 24, 2015

@brson I was thinking about -E, and realized the best fix would probably be to just change the existing sed command to use an extended regex (sed -E 's/pandoc(.exe)? ([0-9]+)\.([0-9]+).*/\2 \3/'); this works on both OS X and Ubuntu for me. How does this patch sound?

@iKevinY iKevinY changed the title Use grep to determine Pandoc version in configure Fix Pandoc version check in configure Feb 25, 2015
Using an extended regex fixes pattern matching on BSD sed.
@brson
Copy link
Contributor

brson commented Mar 5, 2015

@bors: r+ 4349

bors added a commit that referenced this pull request Mar 6, 2015
Executing `configure` seems to create the following error due to how the script [parses Pandoc's version](https://github.com/rust-lang/rust/blob/master/configure#L705):

```text
./configure: line 705: [: pandoc: integer expression expected
./configure: line 705: [: 1.12.4.2: integer expression expected
```

This issue seems to stem from a discrepancy between BSD and GNU versions of sed. This patch changes the sed command to use an extended regex, which works with both flavours of sed.
@bors
Copy link
Collaborator

bors commented Mar 6, 2015

⌛ Testing commit 4349fa4 with merge cd05a57...

@bors
Copy link
Collaborator

bors commented Mar 6, 2015

💔 Test failed - auto-linux-32-opt

@alexcrichton
Copy link
Member

@bors: retry

@bors
Copy link
Collaborator

bors commented Mar 6, 2015

⌛ Testing commit 4349fa4 with merge aba1498...

Manishearth added a commit to Manishearth/rust that referenced this pull request Mar 6, 2015
 Executing `configure` seems to create the following error due to how the script [parses Pandoc's version](https://github.com/rust-lang/rust/blob/master/configure#L705):

```text
./configure: line 705: [: pandoc: integer expression expected
./configure: line 705: [: 1.12.4.2: integer expression expected
```

This issue seems to stem from a discrepancy between BSD and GNU versions of sed. This patch changes the sed command to use an extended regex, which works with both flavours of sed.
@bors
Copy link
Collaborator

bors commented Mar 6, 2015

⛄ The build was interrupted to prioritize another pull request.

@bors
Copy link
Collaborator

bors commented Mar 6, 2015

⌛ Testing commit 4349fa4 with merge 0fa295c...

@bors
Copy link
Collaborator

bors commented Mar 6, 2015

⛄ The build was interrupted to prioritize another pull request.

@bors
Copy link
Collaborator

bors commented Mar 6, 2015

⌛ Testing commit 4349fa4 with merge a514272...

@bors
Copy link
Collaborator

bors commented Mar 6, 2015

⛄ The build was interrupted to prioritize another pull request.

@bors
Copy link
Collaborator

bors commented Mar 6, 2015

⌛ Testing commit 4349fa4 with merge 6193438...

@bors
Copy link
Collaborator

bors commented Mar 6, 2015

⛄ The build was interrupted to prioritize another pull request.

Manishearth added a commit to Manishearth/rust that referenced this pull request Mar 6, 2015
 Executing `configure` seems to create the following error due to how the script [parses Pandoc's version](https://github.com/rust-lang/rust/blob/master/configure#L705):

```text
./configure: line 705: [: pandoc: integer expression expected
./configure: line 705: [: 1.12.4.2: integer expression expected
```

This issue seems to stem from a discrepancy between BSD and GNU versions of sed. This patch changes the sed command to use an extended regex, which works with both flavours of sed.
@bors
Copy link
Collaborator

bors commented Mar 6, 2015

⌛ Testing commit 4349fa4 with merge 639ee30...

bors added a commit that referenced this pull request Mar 6, 2015
Executing `configure` seems to create the following error due to how the script [parses Pandoc's version](https://github.com/rust-lang/rust/blob/master/configure#L705):

```text
./configure: line 705: [: pandoc: integer expression expected
./configure: line 705: [: 1.12.4.2: integer expression expected
```

This issue seems to stem from a discrepancy between BSD and GNU versions of sed. This patch changes the sed command to use an extended regex, which works with both flavours of sed.
@bors
Copy link
Collaborator

bors commented Mar 6, 2015

⛄ The build was interrupted to prioritize another pull request.

bors added a commit that referenced this pull request Mar 6, 2015
Executing `configure` seems to create the following error due to how the script [parses Pandoc's version](https://github.com/rust-lang/rust/blob/master/configure#L705):

```text
./configure: line 705: [: pandoc: integer expression expected
./configure: line 705: [: 1.12.4.2: integer expression expected
```

This issue seems to stem from a discrepancy between BSD and GNU versions of sed. This patch changes the sed command to use an extended regex, which works with both flavours of sed.
@bors
Copy link
Collaborator

bors commented Mar 6, 2015

⌛ Testing commit 4349fa4 with merge 4d716de...

@bors bors merged commit 4349fa4 into rust-lang:master Mar 6, 2015
@iKevinY iKevinY deleted the pandoc-version-check branch October 17, 2015 19:36
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.

6 participants