You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/07/05 14:44:18 UTC

[GitHub] [arrow-rs] tustvold opened a new pull request, #2005: Don't generate empty google.protobuf.rs

tustvold opened a new pull request, #2005:
URL: https://github.com/apache/arrow-rs/pull/2005

   # Which issue does this PR close?
   
   Closes #.
   
   # Rationale for this change
    
   Since https://github.com/tokio-rs/prost/pull/639 was reverted, prost generates some empty files. Lets explicitly make sure this file is expunged.
   
   # What changes are included in this PR?
   
   Automatically deletes the empty file if it is created
   
   # Are there any user-facing changes?
   
   No
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-rs] alamb commented on a diff in pull request #2005: Don't generate empty google.protobuf.rs

Posted by GitBox <gi...@apache.org>.
alamb commented on code in PR #2005:
URL: https://github.com/apache/arrow-rs/pull/2005#discussion_r916893751


##########
arrow-flight/build.rs:
##########
@@ -88,6 +88,13 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
         file.write_all(buffer.as_bytes())?;
     }
 
+    // Prost currently generates an empty file, this was fixed but then reverted
+    // https://github.com/tokio-rs/prost/pull/639
+    let google_protobuf_rs = Path::new("src/sql/google.protobuf.rs");

Review Comment:
   I was thinking something like https://doc.rust-lang.org/stable/std/fs/struct.Metadata.html#method.len
   
   I was worrying about some poor soul in the future who expects the file to be there (because maybe some new version of prost puts something useful in it) but it is mysteriously deleted



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-rs] tustvold commented on a diff in pull request #2005: Don't generate empty google.protobuf.rs

Posted by GitBox <gi...@apache.org>.
tustvold commented on code in PR #2005:
URL: https://github.com/apache/arrow-rs/pull/2005#discussion_r916891037


##########
arrow-flight/build.rs:
##########
@@ -88,6 +88,13 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
         file.write_all(buffer.as_bytes())?;
     }
 
+    // Prost currently generates an empty file, this was fixed but then reverted
+    // https://github.com/tokio-rs/prost/pull/639
+    let google_protobuf_rs = Path::new("src/sql/google.protobuf.rs");

Review Comment:
   We could do, but given it would be a code change to actually `include!()` it in the source code, I'm inclined to think it doesn't matter



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-rs] alamb merged pull request #2005: Don't generate empty google.protobuf.rs

Posted by GitBox <gi...@apache.org>.
alamb merged PR #2005:
URL: https://github.com/apache/arrow-rs/pull/2005


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-rs] alamb commented on a diff in pull request #2005: Don't generate empty google.protobuf.rs

Posted by GitBox <gi...@apache.org>.
alamb commented on code in PR #2005:
URL: https://github.com/apache/arrow-rs/pull/2005#discussion_r916889875


##########
arrow-flight/build.rs:
##########
@@ -88,6 +88,13 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
         file.write_all(buffer.as_bytes())?;
     }
 
+    // Prost currently generates an empty file, this was fixed but then reverted
+    // https://github.com/tokio-rs/prost/pull/639
+    let google_protobuf_rs = Path::new("src/sql/google.protobuf.rs");

Review Comment:
   should we also test that it is actually empty?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org