Ticket #1519 (closed defect: fixed)

Opened 11 years ago

Last modified 11 years ago

generate-racket can't work on windows [2]

Reported by: yed.liu@… Owned by: murphy
Priority: major Milestone:
Component: murphy/protobuf.plt Keywords: windows
Cc: baoti Version: (1 0)
Racket Version:

Description

I.

The proto file request.proto:

message SearchRequest {
  required string query = 1;
  optional int32 page_number = 2;
  optional int32 result_per_page = 3;
}

generate-racket.rkt:

(require (planet murphy/protobuf:1/generator))
(main)

and main.bat:

..\racket generate-racket.rkt

To generate racket:

D:\Program Files (x86)\Racket\ws>protoc --proto_path=. -plugin=protoc-gen-racket=main.bat --racket_out=. request.proto

*Result*:

--racket_out: protoc-gen-racket: Plugin output is unparseable: \r\nD:\\Program F
iles (x86)\\Racket\\ws>..\\racket generate-racket.rkt \r\nz\277\002\n\013reques
t.rktz\257\002#lang racket/base\n;; Generated using protoc-gen-racket v1.0.0\n(r
equire (planet murphy/protobuf:1/syntax))\n\n(define-message-type\n search-reque
st\n ((required primitive:string query 1)\n (optional primitive:int32 page-numb
er 2)\n (optional primitive:int32 result-per-page 3)))\n\n(provide (all-defined
-out))\n

II.

Other proto file service.proto which it use "import":

import "request.proto";
message SearchResponse {
  message Result {
    required string url = 1;
    optional string title = 2;
    repeated string snippets = 3;
  }
  repeated Result result = 1;
}

To compile:

D:\Program Files (x86)\Racket\ws>protoc --proto_path=. --plugin=protoc-gen-racket=main.bat --racket_out=. service.proto

*Result*:

build-path/convention-type: specified convention incompatible with string path e
lement

path element: "/"
convention: 'unix
context...:

C:\Users\habreil\AppData?\Roaming\Racket\planet\300\5.3.6\cache\murphy\protobu

f.plt\1\0\generator.rkt:312:18: for-loop

C:\Users\habreil\AppData?\Roaming\Racket\planet\300\5.3.6\cache\murphy\protobu

f.plt\1\0\generator.rkt:303:9

D:\Program Files (x86)\Racket\collects\racket\port.rkt:125:0: with-output-to-

string

C:\Users\habreil\AppData?\Roaming\Racket\planet\300\5.3.6\cache\murphy\protobu

f.plt\1\0\generator.rkt:295:5: for-loop

C:\Users\habreil\AppData?\Roaming\Racket\planet\300\5.3.6\cache\murphy\protobu

f.plt\1\0\generator.rkt:295:5: for-loop

C:\Users\habreil\AppData?\Roaming\Racket\planet\300\5.3.6\cache\murphy\protobu

f.plt\1\0\generator.rkt:330:0: main

D:\Program Files (x86)\Racket\ws\generate-racket.rkt: [running body]

--racket_out: protoc-gen-racket: Plugin failed with status code 1.

Change History

  Changed 11 years ago by murphy

  • keywords windows added

See also #1510, which referred to the same problem but didn't include any detailed description and seemed bogus.

  Changed 11 years ago by murphy

If anyone can figure out why this doesn't work on Windows, I'm happy to take suggestions how to fix it. The reported error messages make little sense to me, I have no Windows system to test this out and I'm really tempted to suggest installing a proper operating system as a fix for this problem ;-)

  Changed 11 years ago by baoti

  • cc baoti added

This is a version which can work well on windows,
comments with "sean" is the CHANGES.

(define (generate-racket req)
  (define types
    (make-hash))
  (define protos
    (for/hash ([file (in-list (code-generator-request-proto-file req))])
      (register-types! types file)
      (values (file-descriptor-proto-name file)
              file)))
  (with-handlers ([exn:fail:user?
                   (λ (exn)
                     (code-generator-response* #:error (exn-message exn)))])
    (code-generator-response*
     #:file
     (for/list ([proto-file (code-generator-request-file-to-generate req)]
                #:when #t
                [racket-file (in-value (path-replace-suffix proto-file ".rkt"))]
                [proto (in-value (hash-ref protos proto-file))])
       (code-generator-response:file*
        #:name (path->string racket-file)
        #:content
        (with-output-to-string
         (λ ()
           (parameterize ([print-as-expression #f])
             (display "#lang racket/base")
             (newline)
             (display ";; Generated using protoc-gen-racket v1.0.0")
             (newline)
             (pretty-print
              `(require
                (planet murphy/protobuf:1/syntax)
                ,@(for/list ([dep (in-list (file-descriptor-proto-dependency proto))])
                    (;path->string ; sean modify to some-system-path->string at 2013/07/26
                     some-system-path->string
                     (find-relative-path
                      (let-values ([(base name dir?) (split-path racket-file)])
                        (cond
                          [dir?
                           racket-file]
                          [(memq base '(relative #f))
                           "/"]
                          [else
                           ; sean modify at 2013/07/26
                           ;(build-path/convention-type 'unix "/" base)
                           ;(build-path "/" base)
                           (string->some-system-path
                            (some-system-path->string (build-path "/" base))
                            'unix)
                           ]))
                      (let ([target (path-replace-suffix dep ".rkt")])
                        ; sean modify at 2013/07/26
                        ;(build-path/convention-type 'unix "/" target)
                        ;(build-path "/" target)
                        (string->some-system-path
                         (some-system-path->string (build-path "/" target))
                         'unix)
                        ))))))
             (newline)
             (for-each pretty-print (translate-types types proto))
             (newline)
             (pretty-print '(provide (all-defined-out)))))))))))

  Changed 11 years ago by murphy

Thank you for the proposed patch. I've tried to use the same strategy and clean up the code a little in  changeset c4478a0f1c82.

It still works on Unix, but please test the code in the current head of the Mercurial repository on Windows. If everything is functioning as expected, I'll push a new release to PLaneT.

  Changed 11 years ago by baoti

I test  changeset c4478a0f1c82, but it doesn't work.

plugin.proto:

package google.protobuf.compiler;
option java_package = "com.google.protobuf.compiler";
option java_outer_classname = "PluginProtos";

import "google/protobuf/descriptor.proto";

Test it:

F:\Package\Development\proto\protobuf.plt\bin>protoc -I.. --plugin=protoc-gen-racket=protoc-gen-racket.bat --racket_out=../protos/out ../protos/plugin.proto

path->complete-path: convention of first path incompatible with convention of se
cond path

first path: #<unix-path:descriptor.rkt>
second path: "/"
context...:

C:\Users\Administrator\AppData?\Roaming\Racket\planet\300\5.3.4\cache\murphy\p

rotobuf.plt\1\0\generator.rkt:317:18: for-loop

C:\Users\Administrator\AppData?\Roaming\Racket\planet\300\5.3.4\cache\murphy\p

rotobuf.plt\1\0\generator.rkt:308:9

d:\Development\Racket\collects\racket\port.rkt:125:0: with-output-to-string
C:\Users\Administrator\AppData?\Roaming\Racket\planet\300\5.3.4\cache\murphy\p

rotobuf.plt\1\0\generator.rkt:295:5: for-loop

C:\Users\Administrator\AppData?\Roaming\Racket\planet\300\5.3.4\cache\murphy\p

rotobuf.plt\1\0\generator.rkt:336:0: main

F:\Package\Development\proto\protobuf.plt\bin\protoc-gen-racket.rkt: [running

body]

--racket_out: protoc-gen-racket: Plugin failed with status code 1.


And why is "/"? What is it on windows?

I think that for/list just need to product some relative paths like "request.rkt", "../request.rkt", "path/to/request.rkt".
And you can use current-directory and build-path directly, instead of "/" and build-path/convention-type 'unix:

(for/list ([proto-file (code-generator-request-file-to-generate req)]
                #:when #t
                [racket-file (in-value (path-replace-suffix proto-file ".rkt"))]
                [proto
                 (in-value
                  (hash-ref protos proto-file))])
       (code-generator-response:file*
        #:name (path->string racket-file)
        #:content
        (with-output-to-string
                    (path->string
                     (find-relative-path
                      (let-values ([(base name dir?) (split-path racket-file)])
                        (cond
                          [dir?
                           (path->complete-path racket-file "/")]
                          [(memq base '(relative #f))
                           (current-directory)]
                          [else
                           (path->complete-path base "/")]))
                      (path->complete-path
                       (path-replace-suffix (string->some-system-path dep 'unix) ".rkt")
                       "/")))

Test it:

F:\Package\Development\proto\protobuf.plt\bin>protoc -I.. --plugin=protoc-gen-racket=protoc-gen-racket.bat --racket_out=../protos/out ../protos/plugin.proto

Generated plugin.rkt:

#lang racket/base
;; Generated using protoc-gen-racket v1.1
(require (planet murphy/protobuf:1/syntax)
         "..\\google\\protobuf\\descriptor.rkt")

You can replace all "\\" to "/" on windows:

(define (path->string/unix-type path)
  (case (path-convention-type path)
    [(unix) (path->string path)]
    [(windows) (string-replace (path->string path) "\\" "/")]))

Then,

F:\Package\Development\proto\protobuf.plt\bin>protoc -I.. --plugin=protoc-gen-ra
cket=protoc-gen-racket.bat --racket_out=../protos/out ../protos/plugin.proto

generate plugin.rkt:

#lang racket/base
;; Generated using protoc-gen-racket v1.1
(require (planet murphy/protobuf:1/syntax) "../google/protobuf/descriptor.rkt")

I move the directory google to ../protos, and test:

F:\Package\Development\proto\protobuf.plt\bin>protoc -I../protos --plugin=protoc-gen-racket=protoc-gen-racket.bat --racket_out=../protos/out ../protos/plugin.proto

generate plugin.rkt:

#lang racket/base
;; Generated using protoc-gen-racket v1.1
(require (planet murphy/protobuf:1/syntax) "google/protobuf/descriptor.rkt")

The version that comments with "sean" generates (require (planet murphy/protobuf:1/syntax) "/request.rkt")) in some case. Maybe related to "/".

This is my new version:

(define (path->string/unix-type path)
  (case (path-convention-type path)
    [(unix) (path->string path)]
    [(windows) (string-replace (path->string path) "\\" "/")]))

(define (generate-racket req)
  (define types
    (make-hash))
  (define protos
    (for/hash ([file (in-list (code-generator-request-proto-file req))])
      (register-types! types file)
      (values (file-descriptor-proto-name file)
              file)))
  (with-handlers ([exn:fail:user?
                   (λ (exn)
                     (code-generator-response* #:error (exn-message exn)))])
    (code-generator-response*
     #:file
     (for/list ([proto-file (code-generator-request-file-to-generate req)]
                #:when #t
                [racket-file (in-value (path-replace-suffix proto-file ".rkt"))]
                [proto
                 (in-value
                  (hash-ref protos proto-file))])
       (code-generator-response:file*
        #:name (path->string/unix-type racket-file)
        #:content
        (with-output-to-string
         (λ ()
           (parameterize ([print-as-expression #f])
             (display "#lang racket/base")
             (newline)
             (display ";; Generated using protoc-gen-racket v1.1")
             (newline)
             (pretty-print
              `(require
                (planet murphy/protobuf:1/syntax)
                ,@(for/list ([dep (in-list (file-descriptor-proto-dependency proto))])
                    (path->string/unix-type
                     (find-relative-path
                      (let-values ([(base name dir?) (split-path racket-file)])
                        (cond
                          [dir?
                           (path->complete-path racket-file)]
                          [(memq base '(relative #f))
                           (current-directory)]
                          [else
                           (path->complete-path base)]))
                      (path->complete-path (path-replace-suffix dep ".rkt")))))))
             (newline)
             (for-each pretty-print (translate-types types proto))
             (newline)
             (pretty-print '(provide (all-defined-out)))))))))))

Sorry for my bad english. :(


@yed.liu, you should turn off the ECHO of main.bat:

@ECHO OFF
..\racket generate-racket.rkt

  Changed 11 years ago by murphy

The Unix root ("/") is just used as a dummy to make paths complete since find-relative-path can only operate on complete paths.

I think it is unclean to refer to the current-directory or any system specific paths anywhere in the generator code, because the generator never interacts directly with the local file system, it only exchanges messages with the protocol buffer compiler which eventually performs the necessary file system operations!

@baoti: By the way, your comments would be far easier to read if you posted differences against the existing code in the planet package rather than large complete code listings. It always takes extra work to insert your code dumps in the correct place in the source files and pick out the relevant changes using Mercurial's differencing engine. Also the sheer length of the posts already makes this bug tracker entry unwieldy.

  Changed 11 years ago by murphy

I've made another modification to ensure that all the paths have the same conventions. Please check out  changeset 7a0bdbc2b4cf.

I've also set up a virtual Windows installation and verified that this works as expected. By the way, all the indirection through custom launchers and batch files is completely unnecessary if you put Racket and Protoc in your path and install protobuf.plt via standard means. Just running protoc --racket_out=... on your provided example definitions worked like a charm for me.

  Changed 11 years ago by baoti

Thanks for your suggest.

I've checked out  changeset 7a0bdbc2b4cf, and then,
I worked on "F:\Package\Development\proto\protobuf.plt\protos>", this is the output of "tree .":
F:\PACKAGE\DEVELOPMENT\PROTO\PROTOBUF.PLT\PROTOS
├─extend
│ └─protobuf
├─google
│ └─protobuf
│ └─compiler
└─out

I executed protoc --racket_out=out google\protobuf\compiler\plugin.proto, and generated:

#lang racket/base
;; Generated using protoc-gen-racket v1.1
(require (planet murphy/protobuf:1/syntax) "descriptor.rkt")

Does it work as your expected? But in your generated "plugin.rkt", it is "../descriptor.rkt" rather than "descriptor.rkt".

  Changed 11 years ago by murphy

I tested again, regenerating the standard protocol buffer compiler interfaces, the protobuf.plt extension interfaces and your previous examples both under Unix and Windows and I get identical results.

I cannot reproduce the problem you describe with the missing "../" prefix in plugin.rkt.

  Changed 11 years ago by baoti

I added some trace, and found:

The value of proto-file is "google/protobuf/compiler/plugin.proto",

but the result of {{{(path-replace-suffix

(string->some-system-path proto-file 'unix) ".rkt")}}} is "plugin.rkt";

As same as it, dep is "google/protobuf/descriptor.proto", but the result of {{{(path->complete-path

(path-replace-suffix (string->some-system-path dep 'unix) ".rkt")
unix-root)}}} is "/descriptor.rkt".

My DrRacket? is "version 5.3.6 [3m]", and OS is Win7.

follow-up: ↓ 12   Changed 11 years ago by baoti

I test the function on REPL:

> (path-replace-suffix "google/buf/abc.proto" ".rkt")
#<path:google/buf/abc.rkt>
> (path-replace-suffix (string->some-system-path "google/buf/abc.proto" 'unix) ".rkt")
#<unix-path:abc.rkt>
> 

in reply to: ↑ 11   Changed 11 years ago by baoti

> (string->some-system-path "google/buf/abc.proto" 'unix)
#<unix-path:google/buf/abc.proto>
> 

  Changed 11 years ago by murphy

Oh my! This is almost certainly a bug in the Racket standard library itself :-/

I'm going to consult the Racket mailing list about it.

  Changed 11 years ago by baoti

I had send  a pull request to plt/racket on github.
And it had been merged.

  Changed 11 years ago by murphy

  • status changed from new to closed
  • resolution set to fixed

Great! Since that's fixed in the core, I'm uploading a new version of protobuf.plt to PLaneT and closing this bug.

Note: See TracTickets for help on using tickets.