Skip to content
This repository has been archived by the owner on Dec 18, 2017. It is now read-only.

Improve klr #1070

Merged
merged 1 commit into from
Jan 17, 2015
Merged

Improve klr #1070

merged 1 commit into from
Jan 17, 2015

Conversation

ChengTian
Copy link
Contributor

parent #1013 #1071

  • RuntimeBootstrapper already does the following expansion:
    klr /path/to/MyApplication.dll ---> klr --lib /path/to/ MyApplication.
    So we only need to generate --appbase /path/to/ part in this case

(See https://github.com/aspnet/KRuntime/blob/dev/src/klr.hosting.shared/RuntimeBootstrapper.cs#L298)

  • Modifying src/klr.mono.managed/EntryPoint.cs to do the expansion on Mono is much more easier. We can even add --appbase for klr on Mono in EntryPoint.cs if we want it.

int nLastSeparatorIndex = LastPathSeparatorIndex(pszPath);
if (nLastSeparatorIndex < 0)
{
StringCchCopyW(pszParentDir, MAX_PATH, L".");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

strcpy_s() works for char arrays but we need the version for wide char (WCHAR) arrays. The secure version of lstrcpy() is StringCchCopyW():
http://msdn.microsoft.com/en-us/library/windows/desktop/ms647490(v=vs.85).aspx
http://msdn.microsoft.com/en-us/library/windows/desktop/ms647527(v=vs.85).aspx

@ChengTian
Copy link
Contributor Author

Found a bug:
capture1

Fixed in the latest commit. After fix:
capture2

@davidfowl
Copy link
Member

One thing we need to do is merge the help between the lower layer and apphost,

klr -> Shows help for the klr.exe
klr /path/to/project.json -> Shows help for klr.exe Microsoft.Framework.ApplicationHost

The latter is kind of weird so we should try to make this transparent so that klr --help just shows all the options.

@@ -59,6 +59,7 @@ public Task<int> Main(string[] args)
#else
string applicationBaseDirectory = AppContext.BaseDirectory;
#endif
Console.WriteLine("!!! AppBase: " + applicationBaseDirectory);
Copy link
Contributor

Choose a reason for hiding this comment

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

Err, we really want to print three !?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was a temporary backup and that line was for testing & debugging. Please check the latest commit.

@ChengTian
Copy link
Contributor Author

Merged the help info in the latest commit:
capture

Now running ApplicationHost without any argument doesn't give help info:
capture1

Note that some options only applies to ApplicationHost case. If you are running an assembly with an incompatible option:
capture2

@davidfowl
Copy link
Member

Nice!

@ChengTian
Copy link
Contributor Author

I would suggest that we do #1071 in this PR because help info of klr on Mono now is

Usage: klr [options]

Options:
  --appbase <PATH>                 Application base directory path
  --lib <LIB_PATHS>                Paths used for library look-up
  -?|-h|--help                     Show help information
  --version                        Show version information
  --watch                          Watch file changes
  --packages <PACKAGE_DIR>         Directory containing packages
  --configuration <CONFIGURATION>  The configuration to run under
  --port <PORT>                    The port to the compilation server

--appbase should be available on Mono, as the help info suggested.

@ChengTian
Copy link
Contributor Author

Added --appbase for klr on Mono.
Tested with the following test cases, on both Windows and *nix:

klr
klr foo
klr --foo
klr --port
klr --port 8888
klr .\ConsoleApp
klr .\ConsoleApp run
klr --appbase .\ConsoleApp .\ConsoleApp run
klr --port 8888 .\ConsoleApp run
klr --watch .\ConsoleApp run
klr --port 8888 --watch .\ConsoleApp run
klr .\ConsoleApp\bin\Debug\aspnet50\ConsoleApp.dll
klr --appbase .\ConsoleApp .\ConsoleApp\bin\Debug\aspnet50\ConsoleApp.dll
klr --watch .\ConsoleApp\bin\Debug\aspnet50\ConsoleApp.dll

@davidfowl , please check the latest commit and sign off.

- klr /path/App.dll --> klr --appbase /path/ /path/App.dll
- klr /path/project.json run --> klr --appbase /path/
  Microsoft.Framework.ApplicationHost run
- klr /path/ run --> klr --appbase /path/ Microsoft.Framework.ApplicationHost run
- Fix formatted output of WCHAR strings
- Avoid empty string in lib search paths
- Merge help info of klr and ApplicationHost
- Add '--appbase' for klr on Mono
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants