feat(telegraf): Generate sources from scratch - #18271
Conversation
There was a problem hiding this comment.
🟡 Not ready to approve
The generator’s maintainer documentation contains several stale and inaccurate instructions.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Converts Telegraf’s vendor source from a lookaside download to azldev custom generation.
Changes:
- Generates vendored Go dependencies from the upstream source.
- Switches the vendor archive from bzip2 to gzip.
- Refreshes generated sources and lock metadata.
File summaries
| File | Description |
|---|---|
base/comps/telegraf/generate_source_tarball.sh |
Adapts vendor generation for azldev. |
base/comps/telegraf/telegraf.comp.toml |
Defines the custom source generator. |
base/comps/telegraf/telegraf.spec |
Consumes the gzip vendor archive. |
locks/telegraf.lock |
Refreshes the input fingerprint. |
specs/t/telegraf/sources |
Records the generated archive hash. |
specs/t/telegraf/telegraf.spec |
Updates the rendered source reference. |
Review details
Comments suppressed due to low confidence (3)
base/comps/telegraf/generate_source_tarball.sh:54
- The adjacent comment says the output lands next to this script, but
OUT_FOLDERnow points to the absolute custom-source staging directory. This gives maintainers the wrong location when inspecting or invoking the generator.
readonly OUT_FOLDER="/azldev-gen/output"
base/comps/telegraf/generate_source_tarball.sh:36
- The version-bump instructions still name
SRC_TARBALL_URI, but that constant is removed by this change. Following these instructions now requires editing a nonexistent setting and does not mention the custom source's input/output names.
# Bumping the telegraf version: update PKG_VERSION, SRC_TARBALL_URI, and
# SRC_TARBALL_SHA512 below, then update the matching hashes in telegraf.comp.toml.
base/comps/telegraf/generate_source_tarball.sh:60
- The script does not create
${PKG_NAME}-${PKG_VERSION}-vendor.tar.gz; it only stages${OUT_FOLDER}/vendorfor azldev to archive after the script exits. The usage output currently directs users to a file that will not exist when invoking the script itself.
echo "Produces ${PKG_NAME}-${PKG_VERSION}-vendor.tar.gz at ${OUT_FOLDER}." >&2
- Files reviewed: 5/6 changed files
- Comments generated: 1
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
|
/azp run |
|
Azure Pipelines: 3 pipeline(s) were filtered out due to trigger conditions. |
9513a03 to
1fc0059
Compare
There was a problem hiding this comment.
🟡 Not ready to approve
The documented standalone generator usage does not produce the promised archive and may fail writing to /azldev-gen.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Comments suppressed due to low confidence (1)
base/comps/telegraf/generate_source_tarball.sh:60
- The usage text promises that a direct invocation produces the
.tar.gz, but the script only placesvendor/under/azldev-gen/output; azldev performs the later archiving. A maintainer following this documented command gets no named archive (and commonly cannot write under/azldev-genoutside the mock). Either document that this must run through the custom-source workflow or preserve a manual mode that creates the archive.
echo "Expects ${SRC_TARBALL_NAME} in the current working directory." >&2
echo "Produces ${OUT_FOLDER}/vendor for azldev to archive as" >&2
echo "${PKG_NAME}-${PKG_VERSION}-vendor.tar.gz." >&2
- Files reviewed: 5/6 changed files
- Comments generated: 0 new
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
1fc0059 to
ab93cf4
Compare
|
/azp run |
|
Azure Pipelines: 3 pipeline(s) were filtered out due to trigger conditions. |
There was a problem hiding this comment.
🟡 Not ready to approve
The source generator’s documented toolchain and manual invocation do not match its configured execution environment.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Comments suppressed due to low confidence (2)
base/comps/telegraf/generate_source_tarball.sh:38
- The direct invocation documented here is not usable by a normal maintainer: the script now requires the input beside the current directory and writes to the root-owned
/azldev-gen/outputpath. Please document the actualazldevcustom-source invocation, or make the output directory configurable for manual runs.
# Usage:
# ./generate_source_tarball.sh
base/comps/telegraf/generate_source_tarball.sh:32
- The documented canonical toolchain is not the one this generator installs:
origin.mock-packages = ["golang"]resolves to the distro package, currently Go 1.25.8 (specs/g/golang/golang.spec:115-117), rather than Go 1.26.4. Becausego mod vendorcontributes to the pinned source hash, this makes the stated reproduction recipe inaccurate; either arrange for the generator to use 1.26.4 or document the actual toolchain used for the new hash.
This issue also appears on line 37 of the same file.
# Canonical toolchain used to vendor dependencies:
# go : go1.26.4
- Files reviewed: 5/6 changed files
- Comments generated: 0 new
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
No description provided.