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

Fix lockfile generation in workspaces #1236

Closed
12 tasks done
cd-work opened this issue Sep 19, 2023 · 5 comments
Closed
12 tasks done

Fix lockfile generation in workspaces #1236

cd-work opened this issue Sep 19, 2023 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@cd-work
Copy link
Contributor

cd-work commented Sep 19, 2023

Currently workspace generation does not work properly with workspaces in many ecosystems, since it always expects the manifest and lockfile to be generated in the same directory.

The following ecosystems are missing support:

  • Cargo
  • Yarn
  • Npm
  • Pnpm
  • Bundler (no workspaces)
  • .NET
  • Gradle
  • Maven
  • Go
  • Pip (no workspaces)
  • Poetry (no workspaces)
  • Pipenv (no workspaces)
@cd-work cd-work added bug Something isn't working needs triage Needs to be reviewed or assigned labels Sep 19, 2023
@cd-work cd-work self-assigned this Sep 19, 2023
cd-work added a commit that referenced this issue Sep 19, 2023
This patch fixes the lockfile generation for workspaces with npm, yarn,
and pnpm.

See #1236.
cd-work added a commit that referenced this issue Oct 2, 2023
This patch fixes the lockfile generation for workspaces with npm, yarn,
and pnpm.

See #1236.
@cd-work
Copy link
Contributor Author

cd-work commented Oct 4, 2023

Go seems to be a bit special, there is workspace support and when running go install it will place a go.work.sum in the root of the project, however our current lockfile generation uses go get -d, which always generates the go.sum inside the manifest's directory.

So I don't think we need to do anything for Go workspace support, since our solution sidesteps it. Due to our automatic cleanup of generated files, this also shouldn't leave anything behind.

Should we still need it, then the following patch should work:

Patch
    fn lockfile_path(&self, manifest_path: &Path) -> Result<PathBuf> {
        let project_path = manifest_path
            .parent()
            .ok_or_else(|| Error::InvalidManifest(manifest_path.to_path_buf()))?;

        let output =
            Command::new("go").current_dir(&project_path).args(["env", "GOWORK"]).output()?;

        // Ensure command was successful.
        if !output.status.success() {
            let stderr = String::from_utf8_lossy(&output.stderr);
            return Err(Error::NonZeroExit(output.status.code(), stderr.into()));
        }

        // Get workspace root path.
        let stdout = String::from_utf8(output.stdout).map_err(Error::InvalidUtf8)?;
        let workspace_manifest = stdout.trim();

        if workspace_manifest.is_empty() {
            // If no workspace path is set, assume there is no workspace.
            Ok(project_path.join("go.sum"))
        } else {
            let workspace_manifest = PathBuf::from(workspace_manifest);
            let workspace_root = workspace_manifest
                .parent()
                .ok_or_else(|| Error::InvalidManifest(manifest_path.to_path_buf()))?;
            Ok(workspace_root.join("go.work.sum"))
        }
    }

@cd-work
Copy link
Contributor Author

cd-work commented Oct 5, 2023

I've also tested with .NET and confirmed the lockfile is always generated in the manifest directory.

@cd-work
Copy link
Contributor Author

cd-work commented Oct 5, 2023

I've confirmed that Gradle also generates a gradle.lockfile in the manifest directory when executing it in a multi-project workspace.

@cd-work
Copy link
Contributor Author

cd-work commented Oct 5, 2023

Maven takes the -Doutput parameter to specify the location of the effective pom already. This has been confirmed to work in a workspace project.

@cd-work cd-work closed this as completed Oct 5, 2023
@maxrake
Copy link
Contributor

maxrake commented Oct 5, 2023

To add to the record here...

For poetry, there is a history of interest in monorepo support and an open issue to support subprojects, but currently no workspaces.

@maxrake maxrake removed the needs triage Needs to be reviewed or assigned label Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants