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

[WIP] Add --preferreduilang #858

Closed
wants to merge 4 commits into from

Conversation

enricosada
Copy link
Contributor

ref #815

It's the same as internal --LCID but take the language name ( en-US ) instead of locale id ( 1033 )

i cannot add a real test because resources files are not inside this repository ( why not @KevinRansom ?), so the build is only in english

it's possible to test it with

Release\net40\bin\fsc --simulateException:fsc-oom --preferreduilang:en tests\fsharpqa\Source\CompilerOptions\fsc\preferreduilang\validCulture.fs

Microsoft (R) F# Compiler version (private)
Copyright (c) Microsoft Corporation. All Rights Reserved.

warning FS0075: The command-line option '--simulateException' is for test purpos
es only

error FS0193: internal error: Insufficient memory to continue the execution of t
he program.

Unhandled Exception: OutOfMemoryException.

with another language

Release\net40\bin\fsc --simulateException:fsc-oom --preferreduilang:it tests\fsharpqa\Source\CompilerOptions\fsc\preferreduilang\validCulture.fs

Microsoft (R) F# Compiler version (private)
Copyright (c) Microsoft Corporation. All Rights Reserved.

warning FS0075: The command-line option '--simulateException' is for test purpos
es only

error FS0193: internal error: Memoria insufficiente per continuare l'esecuzione
del programma.

Eccezione non gestita: OutOfMemoryException.

@@ -662,6 +674,13 @@ let advancedFlagsBoth tcConfigB =
utf8OutputFlag tcConfigB;
fullPathsFlag tcConfigB;
libFlag tcConfigB;
CompilerOption("preferreduilang", tagString, OptionString (fun s ->
Copy link
Contributor

Choose a reason for hiding this comment

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

@enricosada Is there a reason why you written this option inline? For consistency reasons I would provide it as an own function like the other flags in this list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. I'll refactor as function

@OmarTawfik
Copy link
Contributor

@dotnet-bot test this please

@OmarTawfik
Copy link
Contributor

@enricosada Jenkins now working on master.

@enricosada
Copy link
Contributor Author

ok, done with @mexx feedback, ready to review!

@enricosada enricosada changed the title Add --preferreduilang [WIP] Add --preferreduilang Jan 25, 2016
@enricosada
Copy link
Contributor Author

added wip beacuse i need to rebase

try
let culture = new Globalization.CultureInfo(s)
if (culture.CultureTypes.HasFlag(Globalization.CultureTypes.UserCustomCulture)) then
// Do not use user custom cultures.
Copy link
Contributor

Choose a reason for hiding this comment

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

@enricosada can you elaborate why not to use user custom cultures?

Copy link
Contributor

Choose a reason for hiding this comment

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

Further the error message will be misleading, as the language name will be actually valid, but will be reported as not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see dotnet/roslyn#2388 something about exception with windows 10.
i am trying to use the same implementation as roslyn

@enricosada enricosada changed the title [WIP] Add --preferreduilang Add --preferreduilang Jan 26, 2016
@enricosada
Copy link
Contributor Author

ok, rebased, fixed merge conflicts about error numbers.
For me is ready to review/merge (removed the [WIP])

@enricosada
Copy link
Contributor Author

@dotnet-bot test Jenkins Windows_NT Release please.

@enricosada
Copy link
Contributor Author

nope 😢 /cc @KevinRansom @otawfik-ms

@OmarTawfik
Copy link
Contributor

@dotnet-bot test this please

@OmarTawfik
Copy link
Contributor

@enricosada I'm not sure if we can trigger specific jobs :(

@enricosada
Copy link
Contributor Author

Dotnet/cli can do it. It's possible @mmitche ?
Btw i'll push something tomorrow trigger build

@enricosada
Copy link
Contributor Author

Was worth a try 😄

@mmitche
Copy link
Member

mmitche commented Jan 27, 2016

@enricosada like this: test Jenkins Windows_NT Debug

@enricosada
Copy link
Contributor Author

thx @mmitche ! i tried it but maybe i need push permission or be organization member? dunno 😄

@enricosada
Copy link
Contributor Author

@dotnet-bot test Jenkins Windows_NT Debug

@mmitche
Copy link
Member

mmitche commented Jan 28, 2016

@enricosada Are you a member of the MS org?

@enricosada
Copy link
Contributor Author

@mmitche i am not a member of MS org, it's ok like that.
I was trying because it's an useful command

@mmitche
Copy link
Member

mmitche commented Jan 28, 2016

@enricosada Actually it shouldn't matter in this case. Btw the reason for the long wait times is that the elevated windows image was offline all night. We are clearing that queue now. We are also switching to an auto-scaling version. I'm targeting the elevated image to be one of the first (maybe tomorrow?) so wait times should drop dramatically.

@enricosada
Copy link
Contributor Author

@mmitche thank you very much.
i have seen commit/pr newer than this one built before this pr, so i was thinking the jenkins integration was stuck, maybe this pr was only queued

@TyOverby
Copy link

@dotnet-bot test this please

@enricosada
Copy link
Contributor Author

This pr is ok.
There is a problem with jenkins build /cc @otawfik-ms @TyOverby

"d:\j\workspace\Microsoft_visualfsharp\debug_windows_nt_prtest\tests\fsharpqa\testenv\src\ILComparer\ILComparer.fsproj" (Build target) (1) -> 10:52:45 (CoreCompile target) -> 10:52:45 d:\j\workspace\Microsoft_visualfsharp\debug_windows_nt_prtest\debug\net40\bin\Microsoft.FSharp.targets(155,9): error MSB4062: The "Fsc" task could not be loaded from the assembly d:\j\workspace\Microsoft_visualfsharp\debug_windows_nt_prtest\debug\net40\bin\FSharp.Build.dll. Could not load file or assembly 'FSharp.Core, Version=4.4.1.9055, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a' or one of its dependencies. Strong name validation failed. (Exception from HRESULT: 0x8013141A) Confirm that the <UsingTask> declaration is correct, that the assembly and all its dependencies are available, and that the task contains a public class that implements Microsoft.Build.Framework.ITask. [d:\j\workspace\Microsoft_visualfsharp\debug_windows_nt_prtest\tests\fsharpqa\testenv\src\ILComparer\ILComparer.fsproj]

@enricosada
Copy link
Contributor Author

@dotnet-bot test this please

@enricosada enricosada changed the title Add --preferreduilang [WIP] Add --preferreduilang Feb 9, 2016
@enricosada
Copy link
Contributor Author

i'll rebase to run the whole test suite, my previous local run didnt catch error with response files, so better play safe

set current culture used by compiler for ui like messages

same as --LCID, but take a culture name ( like "it-IT" ) instead of a locale id
@enricosada enricosada force-pushed the add_preferreduilang branch from 5f21c61 to 19fec16 Compare April 26, 2016 23:21
@enricosada
Copy link
Contributor Author

@dotnet-bot test this please

@enricosada
Copy link
Contributor Author

ok, --preferreduilang is implemented under a PREFERRED_UI_LANG compiler directive, in alternative to --lcid. i think was implemented in coreclr branch
atm PREFERRED_UI_LANG is enabled only for coreclr

I'll change this pr to:

  • enable --preferreduilang to all fsc profiles, no need for PREFERRED_UI_LANG
  • change type of tcConfigB.preferredUiLang to CultureInfo atm is string option ( /cc @dsyme is ok? )
  • fsc --lcid set tcConfigB.preferredUiLang, easy because new CultureInfo(lcid) exists (that's what already do)
  • modify write usage of tcConfigB.lcid to set tcConfigB.preferredUiLang instead
  • modify read usage of tcConfig.lcid is used only to set Thread.CurrentThread.CurrentUICulture ( of CultureInfo type )
  • add tests of this pr to test suite, atm no tests for the argument

@KevinRansom
Copy link
Member

@enricosada Let me know when this is ready to pull. And tell me what you want me to do with the test disabling PR.

@enricosada
Copy link
Contributor Author

I need to do work on this one

Test disabling pr is ok

@KevinRansom
Copy link
Member

Hi @enricosada ,

where do we stand with this?

Kevin

@enricosada
Copy link
Contributor Author

i'll close and open a new pr when ready

@enricosada enricosada closed this Jun 6, 2016
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.

7 participants