#757: Settings in code repository #850

salimbouch · 2024-12-05T20:37:30Z

fixes #757
fixes #826

…/IDEasy into settings-in-code-repo

hohwille · 2024-12-09T08:00:50Z

cli/src/main/java/com/devonfw/tools/ide/commandlet/AbstractUpdateCommandlet.java

+        if (!Files.isDirectory(settingsPath.resolve(GitContext.GIT_FOLDER))) {
+          try {
+            settingsPath = settingsPath.toRealPath();
+          } catch (IOException e) {
+            throw new IllegalStateException("Could not resolve settings folder to real path.", e);
+          }
+        }
        gitContext.pull(settingsPath);


If you want to pull here, you need to also traverse the real path up to where the .git directory is.
So settingsPath.getParent() would be required after toRealPath() (or you do a loop upwards until you have found a .git folder).

I found something pretty interesting. When the settings repository is a symlink and I execute a git command in that repository, then the git command works. Even if the repository is just a folder inside a real repository, git commands work there too. With this I think we dont need to differentiate between the two cases (symlink or not), we can directly update the settings repository.

hohwille · 2024-12-09T08:03:35Z

cli/src/main/java/com/devonfw/tools/ide/commandlet/CreateCommandlet.java

@@ -28,6 +36,8 @@ public CreateCommandlet(IdeContext context) {
    super(context);
    this.newProject = add(new StringProperty("", true, "project"));
    this.skipRepositories = add(new FlagProperty("--skip-repositories"));
+    this.codeRepositoryFlag = add(new FlagProperty("--code"));
+    this.codeRepository = add(new StringProperty("", false, "codeRepository"));


We do not need an extra codeRepository since then the user can provide two URLs what will be rather confusing.

hohwille · 2024-12-09T08:08:33Z

cli/src/main/java/com/devonfw/tools/ide/git/GitContext.java

@@ -58,13 +58,20 @@ public interface GitContext {
   * Checks if there are updates available for the Git repository in the specified target folder by comparing the local commit hash with the remote commit
   * hash.
   *
-   * @param repository the {@link Path} to the target folder where the git repository is located. This should be the folder containing the ".git"
-   *     subfolder.
+   * @param repository the {@link Path} to the target folder where the git repository is located.


From a design perspective, GitContext is a generic helper for git.
This story is first of all something specific for what we want to do with our settings.
This method may also be used for other git repos than settings and then overriding and reusing the same .commit.id file will cause trouble.
Either you need to expose the lower level methods to get the local commit ID and the remote commit ID and do the saving and comparing of the .commit.id outside. Besides the fact that an update is applied from our settings and the fact that the remote and the local commit ID differs are two very different things.
Therefore I would recommend not to integrate this here.

I now made the comparison and saving of commit Id more generic and not specific to our settings repository by just providing these as args to these methods and moved to IdeContext

hohwille · 2024-12-09T08:16:08Z

cli/src/main/java/com/devonfw/tools/ide/git/GitContextImpl.java

+      try {
+        repository = repository.toRealPath();
+      } catch (IOException e) {
+        throw new IllegalStateException("Couldn't resolve settings to real path.", e);
+      }


you have duplicated this code fragment in multiple places. I have made some suggestions to improve this.
This already indicates that this should go to a central place and should be reused.
IMHO this feature is something special for our settings and I would recommend to keep the logic out of GitContext.
You could add a getSettingsGitRepository() method to IdeContext that currently would just delegate to getSettingsPath() but could then be extended by you to do this extra logic to find the real git repo with the .git folder.
Also you can then detect if no .git folder was found at all to resolve #826.
IMHO you should then log an error but return null.
In the use-case of #826 we just wanted to check for an update of the settings. If that is not possible, we could still continue regular commands. IMHO commands like --version or --help should not trigger online network activity after all and especially not fail because of that.

hohwille · 2024-12-09T08:21:50Z

cli/src/main/java/com/devonfw/tools/ide/commandlet/CreateCommandlet.java

+    Path codeRepoPath = this.context.getWorkspacePath().resolve(gitUrl.getProjectName());
+    this.context.getGitContext().pullOrClone(gitUrl, codeRepoPath);


Just as a hint. We have currently implemented GitContext.pull such that we provide the exact directory into which to clone the project since we must specify the name. Therefore we explicitly provide the folder to clone into as . and call the git command inside that directory. You can also omit that last argument and let git do the job to determine the project name from the git URL. However, then you also need to find the sub-directory that has been created by git afterwards in order to find the settings to link. Since the workspace was just created and should be empty, this may be very easy but consider this just as a comment to consider or ignore. You approach to explicitly implement getProjectName() should also be fine as long as we do not hit edge-cases that we are not aware yet where git would do a proper job but our code might get the project name wrong...

is there an example where . is used?

hohwille · 2024-12-09T08:27:32Z

cli/src/main/java/com/devonfw/tools/ide/commandlet/CreateCommandlet.java

+      } else {
+        initializeCodeRepository(repoUrl);
+      }
+    }
    super.run();


this will still call updateSettings(). Is your intention to initialize the settings before so it will afterwards update the repo that has just been cloned? IMHO this is still confusing since milliseconds after cloning we do not expect changes and updating again does not make much sense.
It would IMHO make more sense to override updateSettings() method or to extract a new protected method out of updateSettings() that does the cloning part and potentially asks for settings URL. Then you can override only this method here.

done, overrode updateSettings()

coveralls · 2024-12-12T00:44:58Z

Pull Request Test Coverage Report for Build 12299589555

Details

0 of 0 changed or added relevant lines in 0 files are covered.
204 unchanged lines in 7 files lost coverage.
Overall coverage decreased (-0.2%) to 66.914%

Files with Coverage Reduction	New Missed Lines	%
com/devonfw/tools/ide/commandlet/AbstractUpdateCommandlet.java	1	84.13%
com/devonfw/tools/ide/git/GitContext.java	4	0.0%
com/devonfw/tools/ide/git/GitUrl.java	6	75.0%
com/devonfw/tools/ide/commandlet/CreateCommandlet.java	14	61.11%
com/devonfw/tools/ide/context/IdeContext.java	16	59.74%
com/devonfw/tools/ide/git/GitContextImpl.java	41	60.71%
com/devonfw/tools/ide/context/AbstractIdeContext.java	122	60.78%

Totals
Change from base Build 12299210604:	-0.2%
Covered Lines:	6640
Relevant Lines:	9574

💛 - Coveralls

salimbouch and others added 6 commits December 5, 2024 18:04

did a little bit of everything, javadoc and tests missing

84a29cf

added documentation

f73b3fe

tiny changes

6923f71

Merge branch 'main' into settings-in-code-repo

0e09cbe

Update GitContextMock.java

d4a5590

Merge branch 'settings-in-code-repo' of https://github.com/salimbouch…

11f372d

…/IDEasy into settings-in-code-repo

salimbouch self-assigned this Dec 5, 2024

Update GitContextMock.java

5eb994f

hohwille marked this pull request as draft December 6, 2024 09:37

hohwille reviewed Dec 9, 2024

View reviewed changes

salimbouch and others added 8 commits December 9, 2024 18:03

review changes

cb46e0e

review changes

0284234

moved save commit Id method to IdeContext

d8e4e88

review changes

d67de02

removed unused import

094836e

fixed nls tests

7523af6

fixed tests

5b266fe

fixed nls test

5de9a0f

salimbouch marked this pull request as ready for review December 12, 2024 00:47

salimbouch added 5 commits December 12, 2024 01:48

Merge branch 'main' into settings-in-code-repo

a7b5ec2

fixed 826

cbf6cdc

imports

446475f

Update AbstractIdeContext.java

d112979

added check so that tests dont break

ff61a2b

Merge branch 'main' into settings-in-code-repo

845c9d9

salimbouch requested a review from hohwille December 12, 2024 15:51

salimbouch assigned hohwille Dec 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#757: Settings in code repository #850

#757: Settings in code repository #850

salimbouch commented Dec 5, 2024 •

edited

Loading

hohwille Dec 9, 2024

salimbouch Dec 9, 2024

hohwille Dec 9, 2024

salimbouch Dec 9, 2024

hohwille Dec 9, 2024

salimbouch Dec 9, 2024 •

edited

Loading

hohwille Dec 9, 2024

hohwille Dec 9, 2024

salimbouch Dec 12, 2024

hohwille Dec 9, 2024

salimbouch Dec 9, 2024

coveralls commented Dec 12, 2024 •

edited

Loading

		Path codeRepoPath = this.context.getWorkspacePath().resolve(gitUrl.getProjectName());
		this.context.getGitContext().pullOrClone(gitUrl, codeRepoPath);

#757: Settings in code repository #850

Are you sure you want to change the base?

#757: Settings in code repository #850

Conversation

salimbouch commented Dec 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

salimbouch Dec 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Dec 12, 2024 • edited Loading

Pull Request Test Coverage Report for Build 12299589555

Details

💛 - Coveralls

salimbouch commented Dec 5, 2024 •

edited

Loading

salimbouch Dec 9, 2024 •

edited

Loading

coveralls commented Dec 12, 2024 •

edited

Loading