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

Export changed files via builtin environment variables #2935

Merged
merged 9 commits into from
Dec 18, 2023

Conversation

6543
Copy link
Member

@6543 6543 commented Dec 13, 2023

add CI_PIPELINE_FILES to builtin env vars

close #853


Sponsored by Kithara Software GmbH

@6543 6543 added server enhancement improve existing features labels Dec 13, 2023
@woodpecker-bot
Copy link
Collaborator

woodpecker-bot commented Dec 13, 2023

Deployment of preview was successful: https://woodpecker-ci-woodpecker-pr-2935.surge.sh

Copy link
Contributor

@qwerty287 qwerty287 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this fixes #853?

Can you check the comments there, especially #853 (comment) about the memory/performance with many changed files?

Copy link

codecov bot commented Dec 13, 2023

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (23f58fc) 33.89% compared to head (981c4db) 33.87%.

❗ Current head 981c4db differs from pull request most recent head 1671637. Consider uploading reports for the commit 1671637 to get more accurate results

Files Patch % Lines
pipeline/frontend/metadata/environment.go 0.00% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2935      +/-   ##
==========================================
- Coverage   33.89%   33.87%   -0.02%     
==========================================
  Files         219      219              
  Lines       13856    13863       +7     
==========================================
  Hits         4696     4696              
- Misses       8802     8809       +7     
  Partials      358      358              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@anbraten anbraten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't do this as the string can easily be extremely long. Getting the changed files can if needed be done using git already.

@6543
Copy link
Member Author

6543 commented Dec 13, 2023

I'm for adding it.

first:

  1. long env variables dont create any issue
  2. on bigger projects this is a question of efficience (if checked via git in pipeline, it would e.g. add 9min just to checkout the whole code to get the changed files)
  3. we already have the info in metadata and database anyway

@6543 6543 requested a review from anbraten December 13, 2023 14:45
@6543
Copy link
Member Author

6543 commented Dec 13, 2023

if long env var are a concern they posibly can only make issues on windows, I can add a check for this case if desired

linux handle it just fine!

Copy link
Member

@anbraten anbraten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still feels wrong to possibly have those super long env vars, but as it seems to be supported its fine to me.

@anbraten anbraten dismissed their stale review December 14, 2023 09:13

fine for me

@lafriks
Copy link
Contributor

lafriks commented Dec 14, 2023

I would rather see this implemented differently than env variable, maybe having a token available for some special api call to the woodpecker server from the pipeline that would allow it to retrieve such data?

@6543
Copy link
Member Author

6543 commented Dec 15, 2023

So basically: #2291 (comment) ?

@6543

This comment was marked as off-topic.

@confusedsushi
Copy link

For the user it is so convenient to just have that information. Of course he could get this information via git, or maybe later via some sort of API call. But shouldn't you make it for the user as easy as possible? You already have that information, why not forward it to the user? If the user need to bother with some git command, some HTTP API call or some other external tool he will just make mistakes.

@6543
Copy link
Member Author

6543 commented Dec 18, 2023

It now only export if changed files if maxChangedFiles (500) is not exceeded

@6543 6543 requested a review from a team December 18, 2023 18:48
@xoxys
Copy link
Member

xoxys commented Dec 18, 2023

I'm also not sure about the current way of implementation. Why not use the approach from the linked issue and let e.g. plugin-git create a file that contains all changed files, one per line? The current JSON string can't be easily parsed by bash/shell.

@6543
Copy link
Member Author

6543 commented Dec 18, 2023

well first, we do shallow clone by default.
so recover the information the server already has is just realy inefficient.

just to point out some numbers: big repos will rise clone time from ~3min to 12min just to get that info

@6543
Copy link
Member Author

6543 commented Dec 18, 2023

and json can be parsed by jq or yq witch are widely available. powershell (windows) support it natively.

@xoxys
Copy link
Member

xoxys commented Dec 18, 2023

Ah yeah that make sense. Missed this point.

@6543
Copy link
Member Author

6543 commented Dec 18, 2023

I think this fixes #853?

yes it does :) - updated pull description

Copy link
Member

@xoxys xoxys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM.

@6543 6543 enabled auto-merge (squash) December 18, 2023 21:26
@6543 6543 merged commit 936c9bd into woodpecker-ci:main Dec 18, 2023
6 checks passed
@6543 6543 deleted the buildin-env.export-changed-files branch December 18, 2023 21:38
@woodpecker-bot woodpecker-bot mentioned this pull request Dec 17, 2023
1 task
6543 pushed a commit that referenced this pull request Dec 26, 2023
## [2.1.0](https://github.com/woodpecker-ci/woodpecker/releases/tag/2.1.0)
- 2023-12-26

### ✨ Features

- Add pull request closed event
[[#2684](#2684)]
- Add depends_on support for steps
[[#2771](#2771)]
- gitlab: support nested repos
[[#2981](#2981)]
- Support go plugins for forges and agent backends
[[#2751](#2751)]

### 📈 Enhancement

- Show default branch on top
[[#3019](#3019)]
- Support more addon types
[[#2984](#2984)]
- Hide PR tab if PRs are disabled
[[#3004](#3004)]
- Switch to ULID
[[#2986](#2986)]
- Ignore pipelines without config
[[#2949](#2949)]
- Link labels to input and select
[[#2974](#2974)]
- Register Agent with hostname
[[#2936](#2936)]
- Update slogan & logo
[[#2962](#2962)]
- Improve error handling when activating a repository
[[#2965](#2965)]
- Add check for storage where repo/org name is empty
[[#2968](#2968)]
- Update pipeline icons
[[#2783](#2783)]
- Kubernetes refactor
[[#2794](#2794)]
- Export changed files via builtin environment variables
[[#2935](#2935)]
- Show secrets from org and global level
[[#2873](#2873)]
- Only update pipelineStatus in one place
[[#2952](#2952)]
- Rename `engine` to `backend`
[[#2950](#2950)]
- Add linting for `log.Fatal()`
[[#2946](#2946)]
- Remove separate root path config
[[#2943](#2943)]
- init CI_COMMIT_TAG if commit ref is a tag
[[#2934](#2934)]
- Update go module path for major version 2
[[#2905](#2905)]
- Unify date/time dependencies
[[#2891](#2891)]
- Add linting for `any`
[[#2893](#2893)]
- Fix vite deprecations
[[#2885](#2885)]
- Migrate to Xormigrate
[[#2711](#2711)]
- Simple security context options (Kubernetes)
[[#2550](#2550)]
- Changes PullRequest Index to ForgeRemoteID type
[[#2823](#2823)]

### 🐛 Bug Fixes

- Hide queue visualization if nothing to show
[[#3003](#3003)]
- fix and lint swagger file
[[#3007](#3007)]
- Fix IPv6 host aliases for kubernetes
[[#2992](#2992)]
- Fix cli lint throwing error on warnings
[[#2995](#2995)]
- Fix static file caching
[[#2975](#2975)]
- Gitea driver: ignore GetOrg error if we get a valid user.
[[#2967](#2967)]
- feat(k8s): Add a port name to service definition
[[#2933](#2933)]
- Fix error container overflow
[[#2957](#2957)]
- ignore some errors on repairAllRepos
[[#2792](#2792)]
- Allow to restart pipelines that has warnings
[[#2939](#2939)]
- Fix skipped pipelines model
[[#2923](#2923)]
- fix: Add `backend_options` to service linter entry
[[#2930](#2930)]
- Fix flags added multiple times
[[#2914](#2914)]
- Fix schema validation with array syntax for clone and services
[[#2920](#2920)]
- Fix prometheus docs
[[#2919](#2919)]
- Fix podman agent container in v2
[[#2897](#2897)]
- Fix bitbucket org fetching
[[#2874](#2874)]
- Only deploy docs on `main`
[[#2892](#2892)]
- Fix pipeline-related environment
[[#2876](#2876)]
- Fix version check partially
[[#2871](#2871)]
- Fix unregistering agents when using agent tokens
[[#2870](#2870)]

### 📚 Documentation

- [Awesome Woodpecker] added yet another autoscaler
[[#3011](#3011)]
- Add cookbook blog and improve docs
[[#3002](#3002)]
- Replace multi-pipelines with workflows on docs frontpage
[[#2990](#2990)]
- Update README badges
[[#2956](#2956)]
- Update 20-kubernetes.md
[[#2927](#2927)]
- Add release documentation to CONTRIBUTING
[[#2917](#2917)]
- Add nix-attic plugin to the index
[[#2889](#2889)]
- Add usage with Tunnelmole to docs
[[#2881](#2881)]
- Improve code blocks in docs
[[#2879](#2879)]
- Add a blog post
[[#2877](#2877)]
- Add documentation on Kubernetes securityContext
[[#2822](#2822)]
- Add default page to categories
[[#2869](#2869)]
- Use same format for Github docs as used for the other forges
[[#2866](#2866)]

### Misc

- chore(deps): update dependency isomorphic-dompurify to v2
[[#3001](#3001)]
- fix(deps): update dependency @intlify/unplugin-vue-i18n to v2
[[#2998](#2998)]
- Fix go in gitpod
[[#2973](#2973)]
- fix(deps): update module google.golang.org/grpc to v1.60.1
[[#2969](#2969)]
- chore(deps): update docker.io/alpine docker tag to v3.19
[[#2970](#2970)]
- Fix broken gated repos
[[#2959](#2959)]
- fix(deps): update golang (packages)
[[#2958](#2958)]
- Update docker.io/techknowlogick/xgo Docker tag to go-1.21.5
[[#2926](#2926)]
- Update docker.io/golang Docker tag to v1.21.5
[[#2925](#2925)]
- Lock file maintenance
[[#2910](#2910)]
- Update web npm deps non-major
[[#2909](#2909)]
- Update docs npm deps non-major
[[#2908](#2908)]
- Update golang (packages)
[[#2904](#2904)]
- Update module github.com/google/go-github/v56 to v57
[[#2899](#2899)]
- Update dependency marked to v11
[[#2898](#2898)]
- Update dependency vite-svg-loader to v5
[[#2837](#2837)]
- Update golang (packages)
[[#2894](#2894)]
- Update web npm deps non-major
[[#2895](#2895)]
- Update web npm deps non-major
[[#2884](#2884)]
- Update docker.io/woodpeckerci/plugin-docker-buildx Docker tag to
v2.2.1 [[#2883](#2883)]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement improve existing features server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add changed files env var
7 participants