You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/05/31 05:20:46 UTC
[GitHub] [lucene-solr] dsmiley opened a new pull request #1546: SOLR: Use absolute paths for server paths.
dsmiley opened a new pull request #1546:
URL: https://github.com/apache/lucene-solr/pull/1546
Fixes relative path ambiguities when the current working directory is atypical.
I discovered this problem when running Solr standalone from IntelliJ using a CWD that was maybe slightly unexpected. Some relative paths like the core instance dir were getting resolved relative to the CWD instead of the solr home or core root dir. So I think it's preferable to make the major directories of Solr be absolute to avoid this problem. Not worth a CHANGES.txt entry, I think.
BTW I *thought* maybe this might be related to https://issues.apache.org/jira/browse/SOLR-7323 because I was trying to reproduce it but no it's not.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1546: SOLR: Use absolute paths for server paths.
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #1546:
URL: https://github.com/apache/lucene-solr/pull/1546#discussion_r439083930
##########
File path: solr/core/src/java/org/apache/solr/core/CoreDescriptor.java
##########
@@ -182,7 +182,7 @@ public CoreDescriptor(String coreName, CoreDescriptor other) {
*/
public CoreDescriptor(String name, Path instanceDir, Map<String, String> coreProps,
Properties containerProperties, ZkController zkController) {
- this.instanceDir = instanceDir;
+ this.instanceDir = instanceDir.toAbsolutePath();
Review comment:
That makes sense; thanks. I see that all callers send absolute paths already. So I think I should change this to throw an exception if it isn't absolute to protect us from mistakes.
I'll push a new commit here for review. I've never done that for an already merged PR; we'll see how it goes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1546: SOLR: Use absolute paths for server paths.
Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #1546:
URL: https://github.com/apache/lucene-solr/pull/1546#discussion_r432910969
##########
File path: solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java
##########
@@ -77,7 +77,7 @@
String coreName = params.required().get(CoreAdminParams.NAME);
Map<String, String> coreParams = buildCoreParams(params);
CoreContainer coreContainer = it.handler.coreContainer;
- Path instancePath = coreContainer.getCoreRootDirectory().resolve(coreName);
+ Path instancePath;
Review comment:
Just a little change to make the var effectively final, which is clearer.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dsmiley merged pull request #1546: SOLR: Use absolute paths for server paths.
Posted by GitBox <gi...@apache.org>.
dsmiley merged pull request #1546:
URL: https://github.com/apache/lucene-solr/pull/1546
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] janhoy commented on a change in pull request #1546: SOLR: Use absolute paths for server paths.
Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #1546:
URL: https://github.com/apache/lucene-solr/pull/1546#discussion_r439060787
##########
File path: solr/core/src/java/org/apache/solr/core/CoreDescriptor.java
##########
@@ -182,7 +182,7 @@ public CoreDescriptor(String coreName, CoreDescriptor other) {
*/
public CoreDescriptor(String name, Path instanceDir, Map<String, String> coreProps,
Properties containerProperties, ZkController zkController) {
- this.instanceDir = instanceDir;
+ this.instanceDir = instanceDir.toAbsolutePath();
Review comment:
A bit late, but I don't think this is necessary, as all callers will send absolute paths. And if you ever get a relative path, resolving it with `toAbsolutePath()` leads to it being relative to whatever CWD the app is started with, while the typical resolving of relative `instanceDir` is to resolve it relative to CoreContainer#coreRootDirectory.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org