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

Restructure Cookiecutter into simpler per-"type" scripts? #75

Closed
jcbhmr opened this issue Nov 24, 2022 · 5 comments
Closed

Restructure Cookiecutter into simpler per-"type" scripts? #75

jcbhmr opened this issue Nov 24, 2022 · 5 comments

Comments

@jcbhmr
Copy link
Contributor

jcbhmr commented Nov 24, 2022

My honest opinion on the whole Cookiecutter thing:

It's too complicated. There is too much logic concentrated in a template file that belongs in separate scripting files. Cookiecutter and Yeoman (and any project generator really) are great tools, but I don't think they fit the bill the best here.

They work but it's unwieldy to edit them. The files are too big for what they should be. There should be a separate npm-devcontainer-feature.json and one for pip-devcontainer-feature.json if you keep Cookiecutter IMO. They also spread the logic too far apart. You need to reference variables that you declare in JSON and use in Jinja. You need to declare functions separately. You need dependency management. It's a mess.

An alternative is to split each "type" (NPM, pip, apt) into its own Cookiecutter package, but I think a better option is to ditch Cookiecutter as too overkill for simple file generation.

Take Angular for instance. They use ng generate <type> as a way to create new components, interfaces, app shells, and more. I think this approach of script-based generation should be adopted going forward instead of more complex unweidly (and frankly very intimidating) Cookiecutter template logic. I mean have you seen that devcontainer-feature.json file? It is write-once and never touch again code at its finest.

Another argument is that we really need freaking examples of how to do this? Is it really that complicated to just fill out a form in your terminal basically?

Here's an example to really nail home what I am talking about. This is a complete Feature generator script that works! I just wrote it so it might not have proper error handling. It generates an NPM-based feature.

Try it out on Replit! It actually works! https://replit.com/@jcbhmr/SquigglyRepulsiveSolaris#index.js

#!/usr/bin/env -S deno run -A
const npmName = prompt("What is the name of the NPM package?");
const featName = prompt("What should the feature name be?");
const featId = prompt("What should the feature ID be?");
const featDesc = prompt("What should the feature description be?");
const featTestCmd = prompt("What should the test command be?");

const featObject = {
  name: featName,
  description: featDesc,
  id: featId,
  version: "1.0.0",
  documentationURL: `https://github.com/devcontainers-contrib/features/tree/main/src/${featId}#readme`,
  installAfter: ["ghcr.io/devcontainers/features/node:1"],
  options: {
    version: {
      type: "boolean",
      proposals: ["latest"],
      default: "latest",
      description: `Select the version of ${npmName}`,
    },
  },
};
const featJSON = JSON.stringify(featObject, 0, 4);

const featInstallSH = `#!/usr/bin/env bash
set -Eeuo pipefail

if ! command -v npm &>/dev/null; then
  echo "WARN: 'npm' was not found. Installing Node.js + NPM using devcontainer/features src/node/install.sh..."
  curl -fsSL https://raw.githubusercontent.com/devcontainers/features/main/src/node/install.sh | /bin/bash
fi

npm i -g ${npmName}@\${VERSION:-"latest"}
`;

const featTestSH = `#!/usr/bin/env bash
set -Eeuo pipefail

${featTestCmd}
`;

console.log(`src/${featId}/devcontainer-feature.json`);
console.log(featJSON);
console.log(`src/${featId}/install.sh`);
console.log(featInstallSH);
console.log(`test/${featId}/test.sh`);
console.log(featTestSH);

await Deno.mkdir(`src/${featId}`, { recursive: true }); // Same as 'mkdir -p <folder>'.
await Deno.writeTextFile(`src/${featId}/devcontainer-feature.json`, featJSON);
await Deno.writeTextFile(`src/${featId}/install.sh`, featInstallSH);
await Deno.chmod(`src/${featId}/install.sh`, 0o755); // Default of 'chmod +x <file>' is 755.
await Deno.mkdir(`test/${featId}`, { recursive: true });
await Deno.writeTextFile(`test/${featId}/test.sh`, featTestSH);

image

Then the user experience is so much better! You just run the script and it does it for you. You can even "infer" things from the current Git repo like the remote origin URL for documentation URL generation! And it makes it easier to add new features without coupling them to other features (ie. not as many if-statements). I know you can probably do that in Cookiecutter too, but this is one file instead of like 5 files in 3 directories. It's easier to reason about, follow, and maintain. Especially when compared to that monster of an install.sh template.

For instance, here are some generator script ideas:

  • scripts/generate-new-feature-pip.js
  • scripts/generate-new-feature-apt.py
  • scripts/generate-new-feature-deno.sh
  • scripts/generate-new-feature-brew.rb
  • scripts/generate-new-feature-${yourPackageManager}.${yourFavoriteScriptingLanguage}

Heck, you could even commit a C++ file that compiled and ran itself if you really wanted to!

TL;DR: I want a 👍✅ or a 👎⛔ before I invest more time. If I do all the work and the PR doesn't merge, that's not fun for me 😭.


PS: Yes, you can use Python if you're into Python over TypeScript/JavaScript. I personally like the ability of JavaScript + Deno to be self-contained in a single file even if it includes library stuff using HTTP import URLs.

// Using the Prettier code formatter which is relatively large. It's one URL import away!
// https://github.com/denolib/prettier#readme
import { prettier, prettierPlugins } from "https://denolib.com/denolib/prettier/prettier.ts";

const json = JSON.stringify({ my: "object" })
const opts = { parser: "json", plugins: prettierPlugins };
prettier.format(json, opts);
//=> '{ "my": "object" }\n'

Originally posted by @jcbhmr in #69 (comment)

@jcbhmr jcbhmr changed the title Restructure Cookiecutter into simpler per-"type" scripts Restructure Cookiecutter into simpler per-"type" scripts? Nov 26, 2022
@danielbraun89
Copy link
Member

danielbraun89 commented Nov 26, 2022

I agree with most of your points😃 But let me try to explain some of the design decisions (that I may did not got enough time to materialize)

Examples dir

Lets imagine Ihv implemented an np generate style generator.
So a contributor uses to generates a feature and merge it at a certain point of time.
Next week a bug-fix is delivered to the script generator.😮
What you would like to do is to regenerate this feature (and better yet, any other features being generated using the buggy generator) Alas! you cannot reproduce the parameters which were fed into the generator as it was prompted as an interactive input
These changes to the generator happen actually very often, look no further than the recent #84 to see an example of a system wide regeneration of many features.

The flow here is quite different from generators like npx create-react-app and the likes in the sense it's not a one-off action.

But now I clearly see the first issue... calling that dir "examples" is just plain wrong 😄 Its more of a "feature-recipe" dir (or "feature-catalog" or "feature-specs" dir, whatever feels more intuitive).

Cookiecutter implementation
Let me first agree with you. The cookiecutter code is complex, unfriendly, ugly. Basically unmaintainable 😢

The reason I'm currently at peace with this is that I know the "feature.yaml" is the real contract between us and the contributors.
The actual generating code is reduced to just an implementation detail.

If today I have feature.yaml -> cookiecutter-generator -> feature it would be trivial to create a second generator in a later point in time, and switch to feature.yaml -> javscript-generator -> feature

Which brings me to the second issue: currently users activate the generator themselve.
What I would really like to do is: the users open a PR with an additional feature-recepie.yaml only. and its a github action who actually trigger the generating code (be it a cookiecutter based or a javascript based one) and commit the real feature files (devcontainer-feature.json install.sh and test.sh)
(This is kinda taking a page from the GitOps world im also very familiar with from my day job 😅 )

Different generators for each type (pip/ npm/ apt-get)

The motivation to couple those types together were features who needed multiple types together
I think the only feature who does that is localstack which is pip based but needs a certain apt-get package to work. So I agree its quick rare.

However this is an excellent Idea if I take it one step earlier and use these to generate the the yamls themselves

So the chain would be np generate-<npm/pip/apt>-feature -> feature.yaml -> some-generator -> feature

This interactive user input script will make it easy for the user to generate the yaml without having to learn its structure, while still allowing complex manually crafted ones like the mkdocs feature

So a crud "roadmap" would be:

  1. Rename examples dir to something else
  2. Create the github action to auto generate from it
  3. Remove version field from the feature yaml file (should be controlled by the generator version)
  4. Create interactive input yaml generating scripts for common supported use cased
  5. Lastly - get rid of that cookiecutter code in favor of a different implementation 😄

Please allow me sometime to stabilize those before doing any generator changes - I really appreciate the effort you made so far, dont want you to waste time on stuff that might change in future

@danielbraun89
Copy link
Member

Btw this is just for the npm/pip etc features . There are many features that I have manually wrote before the generator (haskell/pulumi/D/zig etc) which are not related. I have stopped adding these when I understood I cannot possible keep up with maintaining more of them...

@jcbhmr
Copy link
Contributor Author

jcbhmr commented Nov 26, 2022

This is the discussion I want to see! 🥇

  1. Rename examples dir to something else
  2. Create the github action to auto generate from it
  3. Remove version field from the feature yaml file (should be controlled by the generator version)
  4. Create interactive input yaml generating scripts for common supported use cased
  5. Lastly - get rid of that cookiecutter code in favor of a different implementation 😄

Right now there are sort of two copies of certain features in the repo. Take Jest for instance.

There's the YAML config file that generates the actual install.sh, test.sh and devcontainer-feature.json, and then there's the actual generated code sitting in the Git repo. This is sort of the same problem with things like TypeScript.

You have a "generated build artifact" in the source control. Ideally, you want some GitHub Action to auto-generate it (like running tsc src/*.ts does in TypeScript projects) like you mentioned.

So, yes, I agree with the summary that eventually the .yml files should be the only thing in source control, with the features being auto-generated in a GitHub Action. That gets a thumbs up 👍 from me!


On another note, I think there might be an alternative. Going back to the ng generate <thing> example, take ng generate component MyComponent. It generates source files that are the same each time. It's sort-of similar to what we are doing here. You might ask "why not just specify components in YAML and generate the source .js files in GitHub Actions?" And you'd be right, you COULD do that. The reason that it's not done is that each component is customized.

This is what ng generate component MyComponent does

CREATE src/app/my-component/my-component.component.css (0 bytes)
CREATE src/app/my-component/my-component.component.html (27 bytes)
CREATE src/app/my-component/my-component.component.spec.ts (635 bytes)
CREATE src/app/my-component/my-component.component.ts (225 bytes)
UPDATE src/app/app.module.ts (418 bytes)
Actual file contents

src/app/my-component/my-component.component.css

/* No content */

src/app/my-component/my-component.component.html

<p>my-component works!</p>

src/app/my-component/my-component.component.spec.ts

import { ComponentFixture, TestBed } from '@angular/core/testing';

import { MyComponentComponent } from './my-component.component';

describe('MyComponentComponent', () => {
  let component: MyComponentComponent;
  let fixture: ComponentFixture<MyComponentComponent>;

  beforeEach(async () => {
    await TestBed.configureTestingModule({
      declarations: [ MyComponentComponent ]
    })
    .compileComponents();

    fixture = TestBed.createComponent(MyComponentComponent);
    component = fixture.componentInstance;
    fixture.detectChanges();
  });

  it('should create', () => {
    expect(component).toBeTruthy();
  });
});

src/app/my-component/my-component.component.ts

import { Component } from '@angular/core';

@Component({
  selector: 'app-my-component',
  templateUrl: './my-component.component.html',
  styleUrls: ['./my-component.component.css']
})
export class MyComponentComponent {

}

src/app/app.module.ts

import { NgModule } from '@angular/core';
import { BrowserModule } from '@angular/platform-browser';

import { AppComponent } from './app.component';
import { MyComponentComponent } from './my-component/my-component.component';

@NgModule({
  declarations: [
    AppComponent,
    MyComponentComponent
  ],
  imports: [
    BrowserModule
  ],
  providers: [],
  bootstrap: [AppComponent]
})
export class AppModule { }

For instance, take Bikeshed. You install it via pip, just like all the other Python dependencies, but it requires a special customization! You can run bikeshed update to fetch the latest stuff from the online database.

image

Anyways, that's one minor example. Over the N number of features, there are bound to be other custom things that need to be added.

I think the best way to do it is to import common features (like check_packages) and still generate actual source files that can be customized

I think that the end-goal looks more like this:

$ scripts/generate-new-feature-npm.js
❓ NPM package name: jest
🟢 CREATE src/jest/devcontainer-feature.json
🟢 CREATE src/jest/install.sh
🟢 CREATE test/jest/test.sh
🔵 UPDATE feature-matrix.json # Or some other index file for the matrix in GitHub Actions

Which creates files that look like this:

src/jest/install.sh

#!/usr/bin/env bash
set -Eeuo pipefail
# Generated from scripts/generate-new-feature-npm.js with npmpackagename=jest
source ../install-header-npm.sh

npm install -g jest

Note how we delegate responsibility to another install-header-npm.sh file! This is kinda what I was getting at. You sort of want the freedom that a raw .sh script gives you to do custom post-insall commands or config, etc. while also pulling in common things that have been centralized so you only need to change one file instead of 40 files for a PR to fix a bug.


So, to summarize, I prefer the option of importing common functionality instead of creating a config manifest file. But really, both are better than what is there now (which is source & build artifacts in the same repo).

👍 Once again, thanks for articulating your process and explaining the why these things where done. Good points, and good discussion!

Also check out #83 for more discussion on that install.sh header idea

@jcbhmr
Copy link
Contributor Author

jcbhmr commented Nov 29, 2022

Here's some of my progress proof-of-concepting my own idea (yeah yeah I need to tidy the commits and stuff): https://github.com/jcbhmr/devcontainers-contrib-features

More discussion in #83

@jcbhmr
Copy link
Contributor Author

jcbhmr commented Dec 1, 2022

🟥🟥🟥 More discussion in #83 🟥🟥🟥

I am closing this because it's more of a discussion, not really an issue to be tracked. Continue discussion in #83 and we can open a new issue if/when we get some concrete "what needs to be done?" tasks.

@jcbhmr jcbhmr closed this as not planned Won't fix, can't repro, duplicate, stale Dec 1, 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

2 participants