jhipster / generator-jhipster

JHipster is a development platform to quickly generate, develop, & deploy modern web applications & microservice architectures.
https://www.jhipster.tech
Apache License 2.0
21.53k stars 4.02k forks source link

Sub generator upgrade: personnal code is deleted #3696

Closed pascalgrimaud closed 8 years ago

pascalgrimaud commented 8 years ago
Overview of the issue

When using the sub generator upgrade, all code in development is deleted.

Motivation for or Use Case

The current code should be kept.

JHipster Version(s)

Master

Reproduce the error

My steps: 1) create folder, and git init: mkdir test && cd test 2) generate a fresh project, all default option: yo jhipster 3) create a simple "hello.txt" file: echo "hello" > hello.txt 4) commit: git add . && git commit -m "Initial commit" 5) then, launch: yo jhipster:upgrade --force 6) the file "hello.txt" has been deleted

I think the problem comes from: starting in a new branch, with only .yo-rc.json and .git (all history), after generation, I got:

diff --git a/hello.txt b/hello.txt
deleted file mode 100644
index ce01362..0000000
--- a/hello.txt
+++ /dev/null
@@ -1 +0,0 @@
-hello
Related issues

Maybe related to https://github.com/jhipster/generator-jhipster/issues/3665

jdubois commented 8 years ago

Indeed, I don't think we should delete the user's files. @lordlothar99 as you have coded it, and probably have more experience than any of us with this sub-generator, can you have a look?

jdubois commented 8 years ago

OK I think we need to have a closer look @pascalgrimaud -> as we use the --force option, it's normal that everything gets deleted!!

lordlothar99 commented 8 years ago

Yes @pascalgrimaud , obviously the purpose of upgrade is to avoid deleting user's code on master branch. On the contrary, user should not ever make any change on jhipster_upgrade branch. The problem here is that code (test file) was added on master before jhipster_upgrade branch was created.... Then, first execution of yo jhipster:upgrade creates jhipster_upgrade branch, and user code is removed (that's normal !). So git merge applies this removal to master. Please note that if we try to avoid deletion of user's code in the upgrade task, it would not be sufficient, cause user can also alter generated code

Actually the best solution I think is to create jhipster_upgrade branch on the very first commit, which should not contain any modified / added user code : 1) create folder, and git init: mkdir test && cd test && git init 2) generate a fresh project, all default option: yo jhipster 3) commit: git add . && git commit -m "Initial commit" 4) create a simple "hello.txt" file: echo "hello" > hello.txt 5) commit: git add . && git commit -m "my first change" 5) then, launch: yo jhipster:upgrade --force : :warning: upgrade sub-generator has to be modified in order to detect that jhipster_upgrade branch doesn't exist, and so it has to create it on "initial commit" (1st in history) 6) the file "hello.txt" is still here !

I propose to make this little change in the sub-generator, and put some information in the documentation indicating that it's strongly recommended to commit all raw generated code when a new project is started. what do you think about that ?

jdubois commented 8 years ago

I'm thinking that you know this better than me, and that I'm trusting you :-) I'll wait for your update before doing the next release!

lordlothar99 commented 8 years ago

ok no pb ! I'm on it

lordlothar99 commented 8 years ago

I have smthg working ; I struggled a bit with synchronization issues ; please have a look at those changes, and ensure async usage is right... obviously I have to read the doc once again !!

jd-carroll commented 8 years ago

Instead of creating branches, we should be creating tags and using those...

On Thursday, June 9, 2016, François Lecomte notifications@github.com wrote:

I have smthg working ; I struggled a bit with synchronization issues ; please have a look at those changes, and ensure async usage is right... obviously I have to read the doc once again !!

— 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/3696#issuecomment-225059372, or mute the thread https://github.com/notifications/unsubscribe/ABzGqGDqUU62XrWKOjncfRv4bx3gl403ks5qKKZMgaJpZM4IxuAo .

lordlothar99 commented 8 years ago

@jd-carroll, tags may be useful to indicate clearly in git history which version was used for the generation, that's right. But branches are mandatory for git to record the diffs between two versions, and so apply only those diffs on modified code during the merge, leaving user's code intact. This is the reason why merges are easier

PierreBesson commented 8 years ago

@jd-carroll, I think having a branch is needed for what we want. The idea is that the jhipster-upgrade branch contains only the code generated by JHipster (without what has been developped by the user) then the app can be safely upgraded in this branch, a diff generated and applied on the user's code (with a git merge).

I prepared a graph to be included in the docs. @lordlothar99 tell me if you like it ?

jhipste-upgrade

You can edit the code here (using Gitgraph.js) http://jsfiddle.net/pbesson/3k1zhsnL/1/

lordlothar99 commented 8 years ago

great graph !! Maybe we should add it in the documentation ? I just made some small changes : http://jsfiddle.net/lordlothar99/tqp9gyu3/

PierreBesson commented 8 years ago

Yes. I will PR it myself to the docs after we agree on a final version in this thread. :smile: I did some minor changes, renaming "Upgrader" to "Updater" http://jsfiddle.net/pbesson/tqp9gyu3/1/

deepu105 commented 8 years ago

@lordlothar99 could you explain what you have done in your PR https://github.com/jhipster/generator-jhipster/pull/3698 I guess I might be missing something. from your earlier commit I thought you suggested that when a new JHipster project is created we should init a git repo and commit the files as well am I correct? what if the user doesnt do this what will happen when they run the upgrade sub generator?

may be as part of initial generation we should do a git init and commit as well from the generator itself.

lordlothar99 commented 8 years ago

yeah sure : when jhipster_upgrade branch doesn't exist yet, master branch holds generated code and maybe code updated/added by the user. It's almost impossible to know if master contains only raw generated code, or altered code. Anyway, as soon as we have a lot of existing projects, let's consider master may contain lots of changes, and we don't have to erase them because of upgrading. that's the whole point.

So, when yo jhipster:upgrade is run for the first time, jhipster_upgrade branch is created. But it's parent should not be the HEAD of master, cause if so, git would consider the generation as a removal of user changes. Instead, I create the jhipster_upgrade branch orphan (no parent), and I generate the whole project BEFORE updating jhipster version. jhipster_upgrade now contains only raw generated code, which may differ from master's HEAD . Anyway, this commit is very useful, because it will be considered as the initial generation of the application. Just after that, I block-merge this commit onto master : (merge with strategy "ours"). So no alteration is made on master's code, but Git now knows that HEAD of master is already up-to-date with everything made on jhipster_upgrade. Everything till here is just like creating a historical reference on master's HEAD, without altering its contents, and preparing upcoming upgrade.

After that, it's normal behavior :

  1. Update of Jhipster version
  2. Regeneration of the whole application on jhipster_upgrade branch : this will obviously create the git diff we need !
  3. Go back on master, and merge : only the diff due to Jhispter's version update will be applied on master, leaving user's changes intact :smile:

Now that I explained, I realize it may be a little complicated ... but I think people don't need to know how it works, as soon as it is !

Obviously, it would be easier if we had a historical Git reference when yo jhipster command is run for the first time, u r right ! I wouldnt have to create it when yo jhipster:upgrade is... But I thought about all projects already generated, and decided to find a way to upgrade them too ! So I don't think it's necessary to initialize Git at first generation. If user does, that's great ; if not, upgrader will work anyway.

deepu105 commented 8 years ago

@lordlothar99 thanks for patiently explaining this :) may be we could add this explanation to the docs as well so that users dont find it as magic :smile_cat: LGTM. I still think it might be a good idea to init a git repo and commit the files after first generation so that for newer projects the whole initial commit creation part will be skipped and save sometime. for old projects your workaround will still be applied.

lordlothar99 commented 8 years ago

Yeah I agree with you on both points. I'll make a PR on doc soon, and maybe let's open a new issue on git initialization?

Le ven. 10 juin 2016 14:36, Deepu K Sasidharan notifications@github.com a écrit :

@lordlothar99 https://github.com/lordlothar99 thanks for patiently explaining this :) may be we could add this explanation to the docs as well so that users dont find it asboth magic 😸 LGTM. I still think it might be a good idea to init a git repo and commit the files after first generation so that for newer projects the whole initial commit creation part will be skipped and save sometime. for old projects your workaround will still be applied.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jhipster/generator-jhipster/issues/3696#issuecomment-225170551, or mute the thread https://github.com/notifications/unsubscribe/AC4SUkHqdMypPVFr1Sfigk9DiLWaJ5MXks5qKVphgaJpZM4IxuAo .

lordlothar99 commented 8 years ago

opened a PR for doc here : https://github.com/jhipster/jhipster.github.io/pull/279 updated gitgraph with recent evolution of upgrade process (link is included in doc) : http://jsfiddle.net/lordlothar99/tqp9gyu3

Invizible commented 8 years ago

Thanks guys, it works fantastic, but there is one problem I faced with. If you used this sub-generator on version 3.4.0 you already have "jhipster_upgrade" branch and sub-generator still erases your code. So I dropped 2 commits this tool made, deleted branch and rerun it again, and everything worked well this time. So I think it's crucial to add such warning in doc, that people should drop old branch before upgrade with new version of sub-generator.

Invizible commented 8 years ago

And I have one question, can we some how modify script to allow changing project options when we upgrade? I mean change DB, or add support for web sockets if previously project was generated without such options? It would be helpful.

deepu105 commented 8 years ago

I dont think it should be part if upgrade. Just change your yo -rc before you run upgrade. On 16 Jun 2016 04:01, "Alex Korobko" notifications@github.com wrote:

And I have one question, can we some how modify script to allow changing project options when we upgrade? I mean change DB, or add support for web sockets if previously project was generated without such options? It would be helpful.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/jhipster/generator-jhipster/issues/3696#issuecomment-226302779, or mute the thread https://github.com/notifications/unsubscribe/ABDlF_BHv77mbJmdfw7BUmys9miNsK51ks5qMFoFgaJpZM4IxuAo .

Invizible commented 8 years ago

@deepu105 I tried it yesterday, updated yo-rc file and ran upgrade tool, but it regenerated project with new options and erased all my changes :(

deepu105 commented 8 years ago

In that case I guess you would have to do it manually. I dont think its easy to do this automatically. I dont have much time to look into it

Thanks & Regards, Deepu

On Thu, Jun 16, 2016 at 3:31 PM, Alex Korobko notifications@github.com wrote:

@deepu105 https://github.com/deepu105 I tried it yesterday, updated yo-rc file and ran upgrade tool, but it regenerated project with new options and erased all my changes :(

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jhipster/generator-jhipster/issues/3696#issuecomment-226409457, or mute the thread https://github.com/notifications/unsubscribe/ABDlFxDz12SbgHZgoRUtimokpcraPN8kks5qMPvqgaJpZM4IxuAo .

gmarziou commented 8 years ago

If you used this sub-generator on version 3.4.0 you already have "jhipster_upgrade" branch and sub-generator still erases your code.

Wondering whether the branch could be named according to target version like "jhipster_upgrade_to_3.4.1" ?

lordlothar99 commented 8 years ago

hi @Invizible ; if your project is opensource, I'm interested in having a look.. even if jhipster_upgrade branch was created before 3.4.1, as soon as it was successfully merged on master, next upgrade would only apply the diff between 3.4.0 and 3.4.1 ; all your changes should have been retained. About changing some options : either a manual update of the .yo-rc.json file, either erasing it, and rerun yo jhipster on cleaned jhipster_upgrade branch.

@gmarziou : we need to use only one branch for all upgrades made on the project, cause this is how we (git, actually) can apply only a diff between Jhipster versions onto user's codebase.

Invizible commented 8 years ago

@lordlothar99 Unfortunately I dropped all extra commits, so you will not see anything interesting. I tried upgrade on demo application which I prepared for lecture about JHipster in our company. Here is the link: https://github.com/Invizible/hipstagram Maybe when I did it, yeomen used old version of upgrade sub-generator, I didn't notice that, I just dropped branch and reran it. Anyway if you say it should work correctly - I'm happy with that :)

About changing some options : either a manual update of the .yo-rc.json file, either erasing it, and rerun yo jhipster on cleaned jhipster_upgrade branch.

Thanks, just tried to add sass

lordlothar99 commented 8 years ago

@Invizible you were right about the deletion of your code ; actually first upgrade (first time jhipster_upgrade branch is created), everything works fine, but from second upgrade, regeneration occurs on wrong branch (my misake !!!). I just opened issue #3757, and submitted PR #3758 with the fix ;)

Invizible commented 8 years ago

@lordlothar99, thanks for fix. And I have one question to you. I have jhipster project, generated with version 3.0.0. So as I understood, upgrade tool generates project in jhipster_upgrade branch with the current version and then installs newest version and generates project again. So since we didn't have this tool in version 3.0.0 we can't upgrade. Can we update the script and at first install locally hipster with version from yo-rc.json, generate, commit and then update hipster globally and locally and then generate project. So that if we have such project, we upgrade hipster globally and upgrade sub-generator becomes available, but since we install older version locally it generates project with correct version and then upgrade

lordlothar99 commented 8 years ago

@Invizible ; upgrade submodule should be able to upgrade a codebase whatever the Jhipster version is (even before 3.4.0)... but you are right : as jhipster update is global, if you have several projects on your computer, you may be face some issues trying to upgrade those without a jhipster_upgrade branch ... I understand your proposal, and I think it would work.. could you plz open an issue ?

Invizible commented 8 years ago

@lordlothar99 yes, sure. Here it is #3761