You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@rya.apache.org by ejwhite922 <gi...@git.apache.org> on 2018/01/16 18:29:37 UTC

[GitHub] incubator-rya pull request #264: RYA-447 Fixed issue building rya.shell unde...

GitHub user ejwhite922 opened a pull request:

    https://github.com/apache/incubator-rya/pull/264

    RYA-447 Fixed issue building rya.shell under Windows.  Loading data from relative path test failed due to malformed user.home paths.

    ## Description
    When building rya.shell under Windows, loading tests fail due to malformed user.home paths.  It appears that Windows file separators ('\') are causing problems when being used as part of a regex replacement operation since they're not being escaped.
    
    ### Tests
    Build/Unit Test
    
    ### Links
    [Jira](https://issues.apache.org/jira/browse/RYA-447)
    
    ### Checklist
    - [ ] Code Review
    - [ ] Squash Commits
    
    #### People To Review
    @meiercaleb
    @kchilton2
    @jessehatfield
    @isper3at
    @DLotts
    @pujav65


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ejwhite922/incubator-rya RYA-447_RyaShellWindowsUserHome

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-rya/pull/264.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #264
    
----
commit bac6418a41cb3b43bcfecfd24f5b84033d3811ab
Author: eric.white <er...@...>
Date:   2018-01-16T18:27:20Z

    RYA-447 Fixed issue building rya.shell under Windows.  Loading data from relative path test failed due to malformed user.home paths.

----


---

[GitHub] incubator-rya issue #264: RYA-447 Fixed issue building rya.shell under Windo...

Posted by pujav65 <gi...@git.apache.org>.

Github user pujav65 commented on the issue:

    https://github.com/apache/incubator-rya/pull/264
  
    I like what Eric did.  Is the issue that the test is failing, or that this shell command just doesn't work in windows?  If the latter, then Eric's approach seems fine.  If the former, then maybe Kevin is right and we should just fix the test to use File.separator() or something like that.  I looked at the underlying implementation of loadStatementsFile, and it delegates to Sesame/OpenRDF. 
    I don't like Eric's suggestion of replaceAll -- its less readable.  


---

[GitHub] incubator-rya issue #264: RYA-447 Fixed issue building rya.shell under Windo...

Posted by asfgit <gi...@git.apache.org>.

Github user asfgit commented on the issue:

    https://github.com/apache/incubator-rya/pull/264
  
    
    Refer to this link for build results (access rights to CI server needed): 
    https://builds.apache.org/job/incubator-rya-master-with-optionals-pull-requests/668/



---

[GitHub] incubator-rya issue #264: RYA-447 Fixed issue building rya.shell under Windo...

Posted by ejwhite922 <gi...@git.apache.org>.

Github user ejwhite922 commented on the issue:

    https://github.com/apache/incubator-rya/pull/264
  
    asfbot build


---

[GitHub] incubator-rya issue #264: RYA-447 Fixed issue building rya.shell under Windo...

Posted by ejwhite922 <gi...@git.apache.org>.

Github user ejwhite922 commented on the issue:

    https://github.com/apache/incubator-rya/pull/264
  
    System.getProperty("user.home") returns paths with backslashes in Windows (i.e. "C:\\Users\\<user.name>" is returned).  The parsing of this fails in a call to String.replaceFirst(String regex, String replacement) since the backslashes aren't escaped as needed for the replacement param string.  The paths used in the unit test don't affect this since System.getProperty("user.home") is what's causing it to fail (called inside RyaCommand.loadData).


---

[GitHub] incubator-rya issue #264: RYA-447 Fixed issue building rya.shell under Windo...

Posted by ejwhite922 <gi...@git.apache.org>.

Github user ejwhite922 commented on the issue:

    https://github.com/apache/incubator-rya/pull/264
  
    @kchilton2 do you have any preference on the suggested solutions?


---

[GitHub] incubator-rya issue #264: RYA-447 Fixed issue building rya.shell under Windo...

Posted by ejwhite922 <gi...@git.apache.org>.

Github user ejwhite922 commented on the issue:

    https://github.com/apache/incubator-rya/pull/264
  
    @kchilton2 @pujav65 
    Both the test and shell command fail in Windows since it will return a path for rootedFile without file separators.  Another option is we could set the path back to its native file separators after the regex replacements are done.
    
    final String userHome = FilenameUtils.separatorsToUnix(System.getProperty("user.home"));
    final Path rootedFile = Paths.get( FilenameUtils.separatorsToSystem(file.replaceFirst("^~", userHome)) );


---

[GitHub] incubator-rya issue #264: RYA-447 Fixed issue building rya.shell under Windo...

Posted by asfgit <gi...@git.apache.org>.

Github user asfgit commented on the issue:

    https://github.com/apache/incubator-rya/pull/264
  
    
    Refer to this link for build results (access rights to CI server needed): 
    https://builds.apache.org/job/incubator-rya-master-with-optionals-pull-requests/664/<h2>Failed Tests: <span class='status-failure'>3</span></h2><h3><a name='incubator-rya-master-with-optionals-pull-requests/org.apache.rya:rya.prospector' /><a href='https://builds.apache.org/job/incubator-rya-master-with-optionals-pull-requests/664/org.apache.rya$rya.prospector/testReport'>incubator-rya-master-with-optionals-pull-requests/org.apache.rya:rya.prospector</a>: <span class='status-failure'>3</span></h3><ul><li><a href='https://builds.apache.org/job/incubator-rya-master-with-optionals-pull-requests/664/org.apache.rya$rya.prospector/testReport/org.apache.rya.prospector.mr/ProspectorTest/testCount/'><strong>org.apache.rya.prospector.mr.ProspectorTest.testCount</strong></a></li><li><a href='https://builds.apache.org/job/incubator-rya-master-with-optionals-pull-requests/664/org.apache.rya$rya.prospector/testReport/org.apache.rya.prospector.service/ProspectorServiceEvalStatsDAOTest/testCount
 /'><strong>org.apache.rya.prospector.service.ProspectorServiceEvalStatsDAOTest.testCount</strong></a></li><li><a href='https://builds.apache.org/job/incubator-rya-master-with-optionals-pull-requests/664/org.apache.rya$rya.prospector/testReport/org.apache.rya.prospector.service/ProspectorServiceEvalStatsDAOTest/testNoAuthsCount/'><strong>org.apache.rya.prospector.service.ProspectorServiceEvalStatsDAOTest.testNoAuthsCount</strong></a></li></ul>



---

[GitHub] incubator-rya issue #264: RYA-447 Fixed issue building rya.shell under Windo...

Posted by kchilton2 <gi...@git.apache.org>.

Github user kchilton2 commented on the issue:

    https://github.com/apache/incubator-rya/pull/264
  
    What does System.getProperty("user.home") look like when you're running on Windows? If it has forward slashes, then the test should also use forward slashes since you will be providing a Windows path, not a Linux one.


---