Skip to content

LUCENE-10259: Fix startup scripts to allow whitespace in path names#472

Merged
uschindler merged 2 commits into
apache:mainfrom
uschindler:jira/LUCENE-10259
Nov 25, 2021
Merged

LUCENE-10259: Fix startup scripts to allow whitespace in path names#472
uschindler merged 2 commits into
apache:mainfrom
uschindler:jira/LUCENE-10259

Conversation

@uschindler
Copy link
Copy Markdown
Contributor

…otherwise won't start on windows if for example extracted to Windows Desktop or "Program Files"
@uschindler uschindler self-assigned this Nov 24, 2021
@rmuir
Copy link
Copy Markdown
Member

rmuir commented Nov 25, 2021

Can we fix scripts header while we are here. /bin/bash is wrong. Let's use /usr/bin/env bash so it works everywhere (such as BSD). Also for the future, I suspect bash is not really needed and Bourne (sh) will suffice.

@uschindler
Copy link
Copy Markdown
Contributor Author

I will check syntax and maybe move to /bin/sh. I am not sure about the $(...) vs the backticks `...`. The latter works with plain bourne, the first I am unsure. It's hard to test as most linux have only bash. I can try with dash.

@uschindler
Copy link
Copy Markdown
Contributor Author

@rmuir the bourne shell does not support $(...command...) and you can only use backticks which can't be nested. I can fix this by writing the first line split into several lines.

@uschindler
Copy link
Copy Markdown
Contributor Author

uschindler commented Nov 25, 2021

Hi i committed a fix which now uses backticks only. It's 2 lines instead of 1 now.

Can anybody check this again with Mac or do i need to start a shell? Maybe @rmuir can check on his BSD box.

@mocobeta
Copy link
Copy Markdown
Contributor

luke.sh works for me on Fedora 34. I will run it on macOS 11

@uschindler
Copy link
Copy Markdown
Contributor Author

When testing, make sure to extract lucene to a directory with whitespace (often a quick "mv"works and revert after testing).

@mocobeta
Copy link
Copy Markdown
Contributor

mocobeta commented Nov 25, 2021

lucene $ tar xzf lucene/distribution/build/release/lucene-10.0.0-SNAPSHOT.tgz -C /tmp/
lucene $ mv /tmp/lucene-10.0.0-SNAPSHOT/ "/tmp/lucene-10.0.0 SNAPSHOT/"
lucene $ /tmp/lucene-10.0.0\ SNAPSHOT/bin/luke.sh 
lucene $ less /tmp/lucene-10.0.0\ SNAPSHOT/bin/luke.sh 
#!/bin/sh

#  Licensed to the Apache Software Foundation (ASF) under one or more
#  contributor license agreements.  See the NOTICE file distributed with
#  this work for additional information regarding copyright ownership.
#  The ASF licenses this file to You under the Apache License, Version 2.0
#  the "License"); you may not use this file except in compliance with
#  the License.  You may obtain a copy of the License at
# 
#      http://www.apache.org/licenses/LICENSE-2.0
# 
#  Unless required by applicable law or agreed to in writing, software
#  distributed under the License is distributed on an "AS IS" BASIS,
#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
#  See the License for the specific language governing permissions and
#  limitations under the License.

MODULES=`dirname "$0"`/..
MODULES=`cd "$MODULES" && pwd`
java --module-path "$MODULES/modules:$MODULES/modules-thirdparty" --add-modules org.apache.logging.l
og4j --module lucene.luke

This works for me on Fedora 34 and macOS 11.

Copy link
Copy Markdown
Member

@rmuir rmuir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@uschindler uschindler merged commit d973e50 into apache:main Nov 25, 2021
@uschindler uschindler deleted the jira/LUCENE-10259 branch November 25, 2021 15:07
asfgit pushed a commit that referenced this pull request Nov 25, 2021
asfgit pushed a commit that referenced this pull request Nov 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants