Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dspace bat file #2544

Open
wants to merge 16 commits into
base: master
from
Open

Dspace bat file #2544

wants to merge 16 commits into from

Conversation

@Flusspferd123
Copy link

Flusspferd123 commented Oct 10, 2019

Removes the "The entered line is too long. Syntax error" error when entering dspace script commands in Windows 10. E.g. "dspace create-administrator"

@AlexanderS

This comment has been minimized.

Copy link
Contributor

AlexanderS commented Oct 10, 2019

Should we remove bin/buildpath.bat? It may be a left over from ancient java versions when the 8.3 naming convention was required.

Note: You removed dspace/config/dspace.cfg and dspace/config/modules/authentication.cfg. Maybe you can rebase your changes, so that you only include the commit "Minor changes to bat file"?

@Flusspferd123

This comment has been minimized.

Copy link
Author

Flusspferd123 commented Oct 10, 2019

I also think that bin/buildpath.bat can be removed. Shall i remove it from this PR?

@tdonohue

This comment has been minimized.

Copy link
Member

tdonohue commented Oct 10, 2019

@Flusspferd123 : Yes, since this PR seems to be removing the (prior) purpose of buildpath.bat, then I'd recommend you remove it in this PR. Otherwise, it's completely unused, but still sitting in the codebase.

@abollini

This comment has been minimized.

Copy link
Member

abollini commented Oct 17, 2019

it could be useful to rework also the "linux" dspace script in a similar way as it will be useful to have a shorter command line also here. Moreover, it is better to keep the two scripts as close as possible

BTW: I just noted that the dspace script still mention an OAI webapp classpath that of course not longer exist

@Flusspferd123

This comment has been minimized.

Copy link
Author

Flusspferd123 commented Oct 21, 2019

Please give this an extra carefull review. The "linux" dspace script now works on a Linux machine. The bat file works for windows. If gitBash is used on Windows the linux" dspace script does not work.
Please also notice: An empty space in the path of a dependency will break this.

Copy link
Member

abollini left a comment

Tested on linux with opendjk. It works,

dspace/bin/dspace Show resolved Hide resolved
@@ -26,7 +26,7 @@ echo Using DSpace installation in: %cd%

REM Build a CLASSPATH including all classes in oai webapp, all libraries in [dspace]/lib and the config folder.
set DSPACE_CLASSPATH=%CLASSPATH%;config;webapps\oai\WEB-INF\classes\

This comment has been minimized.

Copy link
@abollini

abollini Nov 16, 2019

Member

oai|WEB-INF\classes no longer exists

This comment has been minimized.

Copy link
@tdonohue

tdonohue Nov 26, 2019

Member

Agreed, the OAI webapp libraries are no longer needed here (they don't exist anymore). This (and the line before) can be changed to:

REM Build a CLASSPATH including all libraries in [dspace]/lib and the config folder.
set DSPACE_CLASSPATH=%CLASSPATH%;config
Copy link
Member

tdonohue left a comment

👍 I've tested this on Windows 10 and it works well. However, there's one minor thing left to fix. As noted below, the OAI webapp classes can be removed from the DSPACE_CLASSPATH. Once that is fixed, this can be merged.

@Flusspferd123: can you make this one last change? Once that's completed, we can merge this.

@@ -26,7 +26,7 @@ echo Using DSpace installation in: %cd%

REM Build a CLASSPATH including all classes in oai webapp, all libraries in [dspace]/lib and the config folder.
set DSPACE_CLASSPATH=%CLASSPATH%;config;webapps\oai\WEB-INF\classes\

This comment has been minimized.

Copy link
@tdonohue

tdonohue Nov 26, 2019

Member

Agreed, the OAI webapp libraries are no longer needed here (they don't exist anymore). This (and the line before) can be changed to:

REM Build a CLASSPATH including all libraries in [dspace]/lib and the config folder.
set DSPACE_CLASSPATH=%CLASSPATH%;config
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.