Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(9133)

Issue 274530043: DrCov: update genhtml to handle Windows path

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 5 months ago by zhaoqin
Modified:
8 years, 5 months ago
Reviewers:
bruening
CC:
dynamorio-devs_googlegroups.com
Visibility:
Public.

Description

Commit log for first patchset: --------------- DrCov: update genhtml to handle Windows path - convert \ to / in Windows path ---------------

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -0 lines) Patch
M third_party/lcov/genhtml View 1 chunk +2 lines, -0 lines 2 comments Download

Messages

Total messages: 3
zhaoqin
8 years, 5 months ago (2015-11-23 04:48:20 UTC) #1
bruening
If the reason is explained (just for Cygwin perl?) and it doesn't hurt native Windows ...
8 years, 5 months ago (2015-11-23 15:27:51 UTC) #2
zhaoqin
8 years, 5 months ago (2015-11-23 21:47:00 UTC) #3
https://codereview.appspot.com/274530043/diff/1/third_party/lcov/genhtml
File third_party/lcov/genhtml (right):

https://codereview.appspot.com/274530043/diff/1/third_party/lcov/genhtml#newc...
third_party/lcov/genhtml:1556: $filename =~ tr#\\#/#;
On 2015/11/23 15:27:51, bruening wrote:
> Please explain the situation that this is addressing: is it processing a
Windows
> cov file on Linux?  If so, it doesn't seem like this is sufficient.  If the
> script is being run on Windows, which perl type?  Native perl should handle
> backslash just fine.  Is this only for Cygwin perl?  Does native perl also
> handle forward slashes (I would think so but worth a quick test)?  What about
> MSys perl?  Will MSys perl fail due to the drive letter?  Please explain here
> and in the commit log.

It is processing Windows cov file in cygwin, and must use cygwin's per.
Quick test shows Native perl can handle both backslash and forward slash:
Found common filename prefix "d:/src/pdfium"
Writing .css and .png files.

But there are several other problems of using the native perl:
- no pwd command in cmd shell: 'pwd' is not recognized as an internal or
external command,
- genhtml try to create directory using / as dirsep.
  genhtml: ERROR: cannot create directory core/include/fpdfapi!
So we would support cygwin only for now.
will update commit log.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b