You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "thisisnic (via GitHub)" <gi...@apache.org> on 2023/08/17 18:44:10 UTC

[GitHub] [arrow] thisisnic opened a new pull request, #37226: GH-36638: [R] Error with create_package_with_all_dependencies() on Windows [WIP]

thisisnic opened a new pull request, #37226:
URL: https://github.com/apache/arrow/pull/37226

   ### Rationale for this change
   
   Fix path to directory which stops `create_package_with_all_dependencies()` working on Windows
   
   ### What changes are included in this PR?
   
   Character replacement in the script to ensures path uses `/` instead of `\\`
   
   ### Are these changes tested?
   
   TODO: I will test this myself on Windows
   
   ### Are there any user-facing changes?
   
   Nope


-- 
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] amoeba commented on pull request #37226: GH-36638: [R] Error with create_package_with_all_dependencies() on Windows

Posted by "amoeba (via GitHub)" <gi...@apache.org>.
amoeba commented on PR #37226:
URL: https://github.com/apache/arrow/pull/37226#issuecomment-1714788290

   Hey @thisisnic, I just took a look and it does seem like using `normalizePath` like that would be a good fix. However, I wasn't able to reproduce the error reported in https://github.com/apache/arrow/issues/36638 (possibly because of another error). Can you share your steps so I can verify?
   
   I get:
   
   ```
   > arrow::create_package_with_all_dependencies()
   Downloading Arrow source file
   trying URL 'https://cran.rstudio.com/src/contrib/arrow_13.0.0.tar.gz'
   Content type 'application/x-gzip' length 4178847 bytes (4.0 MB)
   downloaded 4.0 MB
   
   Error in arrow::create_package_with_all_dependencies() : 
     Failed to parse versions.txt
   ```
   
   I did test this PR on macOS and just to confirm it still works there (which it does). I left two comments and I'm going to keep setting up my Windows arrow+R dev environment so I can test.


-- 
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] amoeba commented on pull request #37226: GH-36638: [R] Error with create_package_with_all_dependencies() on Windows

Posted by "amoeba (via GitHub)" <gi...@apache.org>.
amoeba commented on PR #37226:
URL: https://github.com/apache/arrow/pull/37226#issuecomment-1718338560

   Good point about not necessarily needing a full dev environment here 🤦‍♂️. I'll test again in a sec and if it works I think this is good to go.


-- 
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] conbench-apache-arrow[bot] commented on pull request #37226: GH-36638: [R] Error with create_package_with_all_dependencies() on Windows

Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #37226:
URL: https://github.com/apache/arrow/pull/37226#issuecomment-1733459715

   After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 7b14b2b2712bc483cd7d14bbc6c38e26d27074ac.
   
   There were no benchmark performance regressions. 🎉
   
   The [full Conbench report](https://github.com/apache/arrow/runs/17097678185) has more details. It also includes information about possible false positives for unstable benchmarks that are known to sometimes produce them.


-- 
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] amoeba commented on pull request #37226: GH-36638: [R] Error with create_package_with_all_dependencies() on Windows

Posted by "amoeba (via GitHub)" <gi...@apache.org>.
amoeba commented on PR #37226:
URL: https://github.com/apache/arrow/pull/37226#issuecomment-1718342934

   I tested your new version of `create_package_with_all_dependencies` on Win 10 and I was able to build the package archive and install from it successfully so this PR looks good to merge now.


-- 
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] thisisnic commented on pull request #37226: GH-36638: [R] Error with create_package_with_all_dependencies() on Windows

Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on PR #37226:
URL: https://github.com/apache/arrow/pull/37226#issuecomment-1708587555

   I've tested this locally, and it seems to work - generates the `.tar.gz` without the error the user experienced.


-- 
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] thisisnic merged pull request #37226: GH-36638: [R] Error with create_package_with_all_dependencies() on Windows

Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic merged PR #37226:
URL: https://github.com/apache/arrow/pull/37226


-- 
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] github-actions[bot] commented on pull request #37226: GH-36638: [R] Error with create_package_with_all_dependencies() on Windows [WIP]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37226:
URL: https://github.com/apache/arrow/pull/37226#issuecomment-1682788032

   :warning: GitHub issue #36638 **has been automatically assigned in GitHub** to PR creator.


-- 
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] thisisnic commented on pull request #37226: GH-36638: [R] Error with create_package_with_all_dependencies() on Windows

Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on PR #37226:
URL: https://github.com/apache/arrow/pull/37226#issuecomment-1713641149

   @amoeba Got time to give this a quick review?


-- 
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] thisisnic commented on pull request #37226: GH-36638: [R] Error with create_package_with_all_dependencies() on Windows

Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on PR #37226:
URL: https://github.com/apache/arrow/pull/37226#issuecomment-1715174160

   > `normalizePath`
   
   
   
   > Hey @thisisnic, I just took a look and it does seem like using `normalizePath` like that would be a good fix. However, I wasn't able to reproduce the error reported in #36638 (possibly because of another error). Can you share your steps so I can verify?
   > 
   > I get:
   > 
   > ```
   > > arrow::create_package_with_all_dependencies()
   > Downloading Arrow source file
   > trying URL 'https://cran.rstudio.com/src/contrib/arrow_13.0.0.tar.gz'
   > Content type 'application/x-gzip' length 4178847 bytes (4.0 MB)
   > downloaded 4.0 MB
   > 
   > Error in arrow::create_package_with_all_dependencies() : 
   >   Failed to parse versions.txt
   > ```
   > 
   > I did test this PR on macOS and just to confirm it still works there (which it does). I left two comments and I'm going to keep setting up my Windows arrow+R dev environment so I can test.
   
   I don't have a Windows arrow+R dev environment set up currently, so I just installed the R package from CRAN, and then pasted my new version of this function into RStudio and ran it; it doesn't rely on any package internals AFAIK so should get the same results.  If you do that, you'll get an error message which will tell you the location of the `versions.txt` file where you can look to see more useful info about whatever error has occurred.


-- 
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