Skip to content

Stop modifying upstream .gitignore file and put build artifacts in debian/.build/upstream #248

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Stop modifying upstream .gitignore file and put build artifacts in debian/.build/upstream #248

wants to merge 2 commits into from

Conversation

ottok
Copy link
Contributor

@ottok ottok commented Feb 19, 2025 •
edited
Loading

See commits for details. This is a re-submission of #230, pending to be merged potentially in the summer of 2025.

Note that these two changes should stay aligned:

NightTsarina
NightTsarina reviewed Feb 20, 2025
View reviewed changes
Copy link
Contributor

@NightTsarina NightTsarina left a comment

Choose a reason for hiding this comment

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

LGTM to removing changes to upstream .gitignore, but maybe we can remove the custom builddirectory too?

template.go Outdated
Comment on lines 303 to 307
fmt.Fprintf(f, "\tdh $@ --builddirectory=debian/build --buildsystem=golang\n")
// Note: The above `--builddirectory=debian/build` will eventually be obsolete
// in 2028+ then the dh-golang version 1.63+ that has
// https://salsa.debian.org/go-team/packages/dh-golang/-/commit/bc16dff5381b668a71fa99c381baba202c34c789
// is in use everywhere

Copy link
Contributor

Choose a reason for hiding this comment

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

Considering dh-golang 1.63 is already in testing, I think we could remove this completely. After all, these tools are meant to be used from sid..

Copy link
Contributor

Choose a reason for hiding this comment

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

The omission of the --builddirectory option is dependent on the debhelper compat level, see the dh-golang(1) man page for the rationale, and debhelper compat 14 is not yet stable.

@guillemj
Copy link
Contributor

LGTM to removing changes to upstream .gitignore, but maybe we can remove the custom builddirectory too?

From the commit reverting the previous change:

While the general idea makes sense and would make things better, this
had some push back on the review and still got merged anyway. It also
conflicts with an old proposal that still is pending to be updated
and re-proposed to try to unify the build paths for all packages at the
dpkg and debhelper layer, via a new dpkg-build-api level. And doing a
migration twice seems wasteful.

Ref: https://lists.debian.org/debian-dpkg/2020/03/msg00003.html

So, I agree with the idea of the change, but probably not in this specific form.

@ottok
Copy link
Contributor Author

ottok commented Feb 23, 2025

In summary, the proposal from 2020 in https://lists.debian.org/debian-dpkg/2020/03/msg00003.html is instead of all these and more:

  - debian/files
  - debian/*substvars
  - debian/<binary-pkgname>/
  - debian/*debhelper.log
  - debian/.debhelper/
  - debian/tmp/

..just have this:

  - debian/.build/

with all artifacts there.

This would certainly help keep the debian/* stuff tidy and help people learning Debian packaging understand clearly what is a build artifact and what is not. However since this hasn't moved since 2020 I am not sure if it makes sense to postpone tidying things up in Go debian/rules while waiting.

ottok added a commit to ottok/dh-make-golang that referenced this pull request May 14, 2025
@ottok
WIP: Debian#248 @ 3473a2a
7338de1
ottok added a commit to ottok/dh-make-golang that referenced this pull request May 14, 2025
@ottok
WIP: Debian#248 @ 3473a2a
3d58077
ottok added 2 commits May 15, 2025 11:45
@ottok
Put build artifacts in debian/.build/upstream to avoid polluting up…
7abe1e9 
…stream sources

Instead of putting build artifacts in `_build/` at the project root,
where they pollute the upstream sources and lure some people to also
modify the upstream `.gitignore` file, put it in the `debian/`
subdirectory along with all other Debian build artifacts.
@ottok
Stop modifying upstream .gitignore file
b3e5c64 
The fact that Debian builds produce extra files in the build directory
is a separate concern and should not be managed by .gitignores in upstream
directory. Anyways, the list is not going to be complete on most packages,
and instead of extending the list, a better practice is to have proper
`make clean` rules in the `debian/rules`, or to simply run `git clean -fdx`
between builds. Additionally, everyone should be using `gbp pq` to update
patches instead of legacy Quilt, so no more `.pc` directories should be
generated.
@ottok ottok force-pushed the fix-artifact-pollution branch from 3473a2a to b3e5c64 Compare May 15, 2025 18:46
@ottok ottok changed the title Stop modifying upstream .gitignore file and put build artifacts in debian/build/ Stop modifying upstream .gitignore file and put build artifacts in debian/.build/upstream May 15, 2025
@ottok ottok requested a review from guillemj May 15, 2025 18:51
@ottok
Copy link
Contributor Author

ottok commented May 15, 2025

Rebased on latest master

ottok added a commit to ottok/dh-make-golang that referenced this pull request May 15, 2025
@ottok
WIP Debian#248
8928059
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NightTsarina NightTsarina NightTsarina left review comments

@guillemj guillemj Awaiting requested review from guillemj

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ottok @guillemj @NightTsarina