Ensure linked-dist is built for lint CI

[somebody1234]

Pull Request Description

Attempts to fix Lint CI by ensuring IDE build artifacts always exist, because TS relies on build.json and linked-dist for typechecking.

Important Notes

None

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

• The documentation has been updated, if necessary.
• Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
• All code follows the
  Scala,
  Java,
  and
  Rust
  style guides. In case you are using a language not listed above, follow the Rust style guide.
• All code has been tested:
  • Unit tests have been written where possible.
  • If GUI codebase was changed, the GUI was tested when built using ./run ide build.

[somebody1234]

wait what?

 INFO ide_ci::program::command: npmℹ️ 
 INFO ide_ci::program::command: npmℹ️ > [email protected] lint
 INFO ide_ci::program::command: npmℹ️ > npx --yes eslint src
 INFO ide_ci::program::command: npmℹ️ 
 INFO ide_ci::program::command: npm⚠️ npm ERR! code ENOTEMPTY
 INFO ide_ci::program::command: npm⚠️ npm ERR! syscall rename
 INFO ide_ci::program::command: npm⚠️ npm ERR! path /runner/_work/enso/enso/app/ide-desktop/node_modules/iconv-lite
 INFO ide_ci::program::command: npm⚠️ npm ERR! dest /runner/_work/enso/enso/app/ide-desktop/node_modules/.iconv-lite-jSVWoePl
 INFO ide_ci::program::command: npm⚠️ npm ERR! errno -39
 INFO ide_ci::program::command: npm⚠️ npm ERR! ENOTEMPTY: directory not empty, rename '/runner/_work/enso/enso/app/ide-desktop/node_modules/iconv-lite' -> '/runner/_work/enso/enso/app/ide-desktop/node_modules/.iconv-lite-jSVWoePl'
 INFO ide_ci::program::command: npm⚠️ 
 INFO ide_ci::program::command: npm⚠️ npm ERR! A complete log of this run can be found in:
 INFO ide_ci::program::command: npm⚠️ npm ERR!     /home/ci/.npm/_logs/2023-05-05T20_02_39_594Z-debug-0.log

[somebody1234]

so this doesn't work properly. now lint ci is uploading artifacts...
it is green, and only takes ~12m including build, which doesn't even sound too bad...
... but if lint ci is going to upload artifacts every time that's probably not a good thing.

i'm explicitly running ide build but afaict only ide upload is supposed to upload artifacts...?

                let ide_cli = Cli::parse_from([
                    "./run",
                    "ide",
                    "build",
                    "--skip-version-check",
                    "--skip-wasm-opt",
                    "--wasm-profile=dev",
                    "--backend-source",
                    "release",
                    "--backend-release",
                    "nightly",
                ]);

... ah, never mind, ide unconditionally uploads an artifact. in this case it shouldn't be too bad as lint ci should only build ide if build.json or linked-dist are missing

enso/build/cli/src/lib.rs

Lines 565 to 586 in 608c5d8

	pub fn build_ide(
	&self,
	params: arg::ide::BuildInput,
	) -> BoxFuture<'static, Result<ide::Artifact>> {
	let arg::ide::BuildInput { gui, project_manager, output_path, electron_target } = params;
	let input = ide::BuildInput {
	gui: self.get(gui),
	project_manager: self.get(project_manager),
	version: self.triple.versions.version.clone(),
	electron_target,
	};
	let target = Ide { target_os: self.triple.os, target_arch: self.triple.arch };
	let build_job = target.build(&self.context, input, output_path);
	async move {
	let artifacts = build_job.await?;
	if is_in_env() {
	artifacts.upload_as_ci_artifact().await?;
	}
	Ok(artifacts)
	}
	.boxed()
	}

[somebody1234]

i'm just going to assume the iconv-lite issue was a one-off thing

[somebody1234]

not sure how i feel about this solution but it seems to work?

[somebody1234]

again though, not familiar with build so idk if, say, ./run gui is better, or whether this should use a different source (probably...), or whether there's a better way to do this in general

i tried using current-ci-run as the source but that seemed to break CI, i'm assuming because the artifact needs to have already been built. so instead I just used the regular command-line that everyone uses.

i think it's pretty hacky, but it's the only way i can see to build ide/gui, since manually constructing the arguments to pass to handle_ide is definitely not an option

[mwu-tow]

@somebody1234

i'm explicitly running ide build but afaict only ide upload is supposed to upload artifacts...?

To clarify the build-script nomenclature: "artifact" is something uploaded to a CI run (which shows in the summary when run is complete), while "asset" is a file that is uploaded to a release.
ide upload uploads an asset to a release, it is meant to be used only as a part of the release pipeline. (Create release, upload assets, publish the release.)
All builds will upload the artifact by default (see below).

... ah, never mind, ide unconditionally uploads an artifact. in this case it shouldn't be too bad as lint ci should only build ide if build.json or linked-dist are missing

This could be adjusted, but the core assumption in the CI was that we want to avoid duplicating any heavy jobs. As such, if something is built, it will be uploaded as an artifact, so any other jobs that require build artifacts can use it.

i tried using current-ci-run as the source but that seemed to break CI, i'm assuming because the artifact needs to have already been built. so instead I just used the regular command-line that everyone uses.

The current-ci-run downloads the artifact from the ongoing run. (In contrast with ci-run that works only with completed CI run's artifacts.) To use it reliably, you need to have dependencies between CI jobs, so if a job needs an artifact, the job that provides the artifact is completed beforehand.

---

The core issue seems to be that the lint command was never assumed to require having the GUI built. The lint was supposed to be a (relatively) quick check, like cargo check vs cargo build. Why do we need to build the whole IDE just to type-check the TS code? To me this looks like the core problem.

---

Also, if you have any questions about the build script, don't hesitate to ask me on our Discord.

[somebody1234]

@mwu-tow the issue is that typescript depends on linked-dist and build.json to be present, since it imports from both of those locations. there is an alternative PR open that imports the types from the source files instead:
#6583

it's worth noting that i think linked-dist and build.json are only absent on clean runners, which explains why they succeeded on the original PR: link to original successful lint action

re: having to build the whole IDE, some notes:

• technically we only need build.json and linked-dist
• i definitely think that a restructure would be a better solution; it is very weird to have to import * as linkedDist from '../../../../somewhere/outside/of/root/npm/workspace'. i'm not familiar enough with our setup (especially on the ensogl/pack side to know a good solution to this though

[wdanilo]

Please talk with @mwu-tow about proper implementation:

1. I don't think that hardcoding flags like that is a good idea.
2. I'm not sure whether this will not drastically increase the CI time.

Anyway, I believe that @mwu-tow is the best person to chat with and discuss the proper solution here.

[somebody1234]

1. definitely agree, will do
2. the idea was it'd only have to run on clean runners, so 0 minutes extra most of the time, 7 minutes extra the rest of the time. i didn't actually test that though, rerunning lint CI might have been a good idea

[somebody1234]

superseded by #6603