jhipster upgrade command failed for project with Indonesian language generated prior version v6.1.0

l7777777b commented 5 years ago

Overview of the issue

jhipster upgrade command failed for project with indonesian language generated before version v6.1.0

stacktrace:

ERROR! Error: Unsupported language "id" passed as argument to language generator.
Supported languages:
  al    (Albanian)
  ar-ly (Arabic (Libya))
  hy    (Armenian)
  by    (Belorussian)
  bn    (Bengali)
  ca    (Catalan)
  zh-cn (Chinese (Simplified))
  zh-tw (Chinese (Traditional))
  cs    (Czech)
  da    (Danish)
  nl    (Dutch)
  en    (English)
  et    (Estonian)
  fa    (Farsi)
  fi    (Finnish)
  fr    (French)
  gl    (Galician)
  de    (German)
  el    (Greek)
  hi    (Hindi)
  hu    (Hungarian)
  in    (Indonesian)
  it    (Italian)
  ja    (Japanese)
  ko    (Korean)
  mr    (Marathi)
  my    (Myanmar)
  pl    (Polish)
  pt-br (Portuguese (Brazilian))
  pt-pt (Portuguese)
  ro    (Romanian)
  ru    (Russian)
  sk    (Slovak)
  sr    (Serbian)
  es    (Spanish)
  sv    (Swedish)
  tr    (Turkish)
  ta    (Tamil)
  te    (Telugu)
  th    (Thai)
  ua    (Ukrainian)
  uz-Cyrl-uz (Uzbek (Cyrillic))
  uz-Latn-uz (Uzbek (Latin))
  vi    (Vietnamese)

    at Environment.error (C:\Users\comp\AppData\Roaming\npm\node_modules\generator-jhipster\node_modules\yeoman-environment\lib\environment.js:176:40)
    at module.exports.error (C:\Users\comp\AppData\Roaming\npm\node_modules\generator-jhipster\generators\generator-base.js:1427:18)
    at shelljs.exec (C:\Users\comp\AppData\Roaming\npm\node_modules\generator-jhipster\generators\upgrade\index.js:134:23)
    at C:\Users\comp\AppData\Roaming\npm\node_modules\generator-jhipster\node_modules\shelljs\src\exec.js:127:9
    at ChildProcess.exithandler (child_process.js:301:5)
    at ChildProcess.emit (events.js:189:13)
    at maybeClose (internal/child_process.js:970:16)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:259:5)
Emitted 'error' event at:
    at Environment.error (C:\Users\comp\AppData\Roaming\npm\node_modules\generator-jhipster\node_modules\yeoman-environment\lib\environment.js:178:15)
    at module.exports.error (C:\Users\comp\AppData\Roaming\npm\node_modules\generator-jhipster\generators\generator-base.js:1427:18)
    [... lines matching original stack trace ...]
    at Process.ChildProcess._handle.onexit (internal/child_process.js:259:5)

Motivation for or Use Case

cannot update project to v6.1.0

Reproduce the error

create project with v6.0.1 and add Indonesian language translation and also as native language
update generator-jhipster to v6.1.0
run jhipster upgrade

Suggest a Fix

I haven't going through the code, but i assumed it's caused by there's no trace of "id" since all of it was replaced to "in" on v6.1.0 release. It should've still recognized the "id" while upgrading for backward compatibility, and replace it to "in" after everything is completed.

Replacing the existing language json config manually to "in" after the error and do another jhipster upgrade cause another local changes error because of previous partially updated files, so i must delete node_modules directory and use jhipster command to do the upgrade.

JHipster Version(s)

C:\Users\comp\Downloads\jh\test
`-- generator-jhipster@6.1.0

JHipster configuration

JHipster configuration, a `.yo-rc.json` file generated in the root folder

.yo-rc.json file

{
  "generator-jhipster": {
    "databaseType": "sql",
    "devDatabaseType": "mariadb",
    "enableHibernateCache": true,
    "enableSwaggerCodegen": false,
    "enableTranslation": true,
    "jhiPrefix": "test",
    "languages": [
      "id",
      "zh-cn",
      "zh-tw",
      "en"
    ],
    "messageBroker": false,
    "nativeLanguage": "id",
    "packageName": "com.test.test",
    "packageFolder": "com/test/test",
    "prodDatabaseType": "mariadb",
    "searchEngine": false,
    "serviceDiscoveryType": "eureka",
    "skipClient": true,
    "testFrameworks": [
      "gatling",
      "cucumber"
    ],
    "websocket": false,
    "baseName": "test",
    "applicationType": "microservice",
    "buildTool": "gradle",
    "jhipsterVersion": "6.0.1",
    "skipUserManagement": true,
    "clientPackageManager": "npm",
    "authenticationType": "jwt",
    "cacheProvider": "hazelcast",
    "serverPort": "8081",
    "entitySuffix": "",
    "dtoSuffix": "DTO",
    "otherModules": []
  }
}

[v] Checking this box is mandatory (this is just to show you read everything)

eapriansyah commented 5 years ago

This error cause, Indonesian language change from id to in. See in yo.rc. it's still id.

"languages": [ "id", "zh-cn", "zh-tw", "en" ]

l7777777b commented 5 years ago

Yes, that's why i think its not a proper upgrade if we need to manually edit that file before doing the upgrade, especially there's no specific announcement about this change and the effect on upgrade. The jhipster upgrade command should've done everything automatically.

pascalgrimaud commented 5 years ago

There are/were a lot of problem with the upgrade sub generator.

It was coded by someone who didn't contribute to the project any more
It is not tested in our CI as it is too specific and really hard to test
each release, we have some bug with it
I already suggested to delete this sub generator but some users want to keep it
It probably works well for little project, or POC. But for big projects, I personnaly recommand to not use it

So honestly, I don't know what to do with this sub gen and with this ticket

mraible commented 5 years ago

I love the idea behind it and I’ve used it with success myself. As a sponsor, I’d like to see more bug bounties geared towards it.

On Jun 14, 2019, at 12:33 PM, Pascal Grimaud notifications@github.com wrote:

There are/were a lot of problem with the upgrade sub generator.

It was coded by someone who didn't contribute to the project any more It is not tested in our CI as it is too specific and really hard to test each release, we have some bug with it I already suggested to delete this sub generator but some users want to keep it It probably works well for little project, or POC. But for big projects, I personnaly recommand to not use it So honestly, I don't know what to do with this sub gen and with this ticket

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/jhipster/generator-jhipster/issues/9908?email_source=notifications&email_token=AAAELZDARNDY2TD5KAIDV5DP2PW65A5CNFSM4HXWZEY2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXYH6Y#issuecomment-502236155, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAELZHG4NO7NDSEVIW7643P2PW65ANCNFSM4HXWZEYQ.

pascalgrimaud commented 5 years ago

Yes agree with you Matt about the value of this. That's why I think it should go to a separate project, as the upgrade sub gen is simply a list of git commands. It can be replaced by a SH or Bash, and should not depend on a specific version of JHipster release.

Like this, we can't do anything until the next release. And if we manage to find a fix, after a new release, we can't know if it will work well.

The best should be to go to a specific project to not rely on JHipster releases

murdos commented 5 years ago

I personally use (including on big projects) and like the upgrade subgen. I think its place is here, in generator-jhipster where it provides a lot of value, and is well integrated with jhipster, including blueprints.

However this subgenerator is not magic. It can't deal with all breaking change. And c67d8aeeffdabc130e13a726fd5e14f6fbf5e0e5 is such a breaking change. I'm afraid there's nothing we can do right now easily.

We have a cleanup mechanism (that is currently not related to upgrade) that coud be used to deal with such changes, however I wonder how it's supposed to work. Indeed it doesn't really work as previously analyzed, since it seems that the .yo-rc.json is updated before the cleanup code run.

deepu105 commented 5 years ago

May it would be much neater if its moved to the CLI instead of generator like the import jdl part. Since in CLI we don't have any yeoman limitations and the code can be much simpler. Also once in CLI we can use advanced node features if needed

On Sat, 15 Jun 2019, 4:27 pm Aurélien Mino, notifications@github.com wrote:

I personally use (including on big projects) and like the upgrade subgen. I think its place is here, in generator-jhipster where it provides a lot of value, and is well integrated with jhipster, including blueprints.

However this subgenerator is not magic. It can't deal with all breaking change. And c67d8ae https://github.com/jhipster/generator-jhipster/commit/c67d8aeeffdabc130e13a726fd5e14f6fbf5e0e5 is such a breaking change. I'm afraid there's nothing we can do right now easily.

We have a cleanup https://github.com/jhipster/generator-jhipster/blob/master/generators/cleanup.js mechanism (that is currently not related to upgrade) that coud be used to deal with such changes, however I wonder how it's supposed to work. Indeed it doesn't really work as previously analyzed https://github.com/jhipster/generator-jhipster/issues/9204#issuecomment-461074682, since it seems that the .yo-rc.json is updated before the cleanup code run.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/jhipster/generator-jhipster/issues/9908?email_source=notifications&email_token=AAIOKF3MDUMNSGEFXMPQIADP2T35VA5CNFSM4HXWZEY2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXYZH6Q#issuecomment-502371322, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIOKF5ZF7CD2MLKL3LIGCDP2T35VANCNFSM4HXWZEYQ .

pvliss commented 5 years ago

Also agree with @murdos that it is very useful and would vote for it to stay. @deepu105 might also be right that moving it to CLI could make it easier to maintain. What I think is missing here currently, in order to support such use cases, is a step for executing pre-upgrade tasks. So before re-generating the app using the latest version we should have updated the .yo-rc.json file accordingly.

pascalgrimaud commented 5 years ago

Yes, I know a lot of people want to keep it. That's why we should find an official maintainer for this

deepu105 commented 5 years ago

I propose we first move it to CLI and simplify a bit

Thanks & Regards, Deepu

On Tue, Jun 18, 2019 at 6:31 PM Pascal Grimaud notifications@github.com wrote:

Yes, I know a lot of people want to keep it. That's why we should find an official maintainer for this

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jhipster/generator-jhipster/issues/9908?email_source=notifications&email_token=AAIOKF5U64X7DOJJ5AWWSO3P3EEWFA5CNFSM4HXWZEY2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODX7GT6Q#issuecomment-503212538, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIOKF32IZAPNFBFHA3GB6DP3EEWFANCNFSM4HXWZEYQ .

pascalgrimaud commented 5 years ago

So can I assign the ticket to one of you ? @pvliss or @murdos ?

pvliss commented 5 years ago

I can give it a shot, except if @murdos wants to do it.

murdos commented 5 years ago

@pvliss : you can go ahead, I don't have time to work on it currently

pascalgrimaud commented 5 years ago

@pvliss : FYI, just added a bounty on this

pvliss commented 5 years ago

Thanks @pascalgrimaud. I have alredy done some work related to this in #10037(needs conflict resolution) and more specifically in commit https://github.com/jhipster/generator-jhipster/pull/10037/commits/69b5a027f1360cc5cd2a60ca88a6d1677300a9d2.

Although with the fix #10226 provided by @kaidohallik maybe we could move this pre-upgrade logic into the language sub-gen itself. Will need to check. The only problem I can think of right now is that a change is needed in the config file before actual generation and we need to check if the languages config array is used before the language sub-gen is invoked.

@kaidohallik are you interested in checking this since you also fixed #10226?

PierreBesson commented 5 years ago

Somehow I totally missed this issue. The reason I changed the locale name was because of a bug in Java specifically related to parsing the correct indonesian locale (see: https://github.com/jhipster/generator-jhipster/issues/9494#issuecomment-480386162 and https://bugs.openjdk.java.net/browse/JDK-6457127). This is a super specific bug and IMO not worth fixing. Also it looks like the discussion moved away from the Original Poster's bug. So IMO we should close this issue.

pvliss commented 5 years ago

@PierreBesson Guess you are right. Not many users should stubmle on this so not sure if its worth the effort and the manual fix is easy as well. Only thing is the end-user experience when upgrading as this change was between minor versions.

pascalgrimaud commented 4 years ago

It is opened since 4 months. How can we advance on this ticket ? cc @pvliss @murdos ?

pascalgrimaud commented 4 years ago

Closing this as it is opened for too long. And as Pierre said, the discussion moved away from the Original Poster's bug.

jhipster / generator-jhipster