Skip to content

Completely separate the release attachment and issue / pull request attachment feature#37080

Open
a1012112796 wants to merge 1 commit intogo-gitea:mainfrom
a1012112796:zzc/dev/splite_release_attachments_feature
Open

Completely separate the release attachment and issue / pull request attachment feature#37080
a1012112796 wants to merge 1 commit intogo-gitea:mainfrom
a1012112796:zzc/dev/splite_release_attachments_feature

Conversation

@a1012112796
Copy link
Copy Markdown
Member

@a1012112796 a1012112796 commented Apr 2, 2026

After a careful analysis of the Gitea attachment feature related code, I found that initially, the two were simply reused directly. At least since #12465, people have been making them different due to various requirements. After #36697, I think maybe it's a time to completely separate them.

  • Add a configuration item [repository.release] -> ENABLE_ATTACHMENT to independently control whether to enable release attachments.
  • Add a configuration item section [repository.release.attachment] to configure an independent storage method for release attachments (optional).

Configuration examples:

Enable both release attachment and issue / pull request attachment, configure storage independently.

[repository.release.attachment]
PATH = release-attachment

Enable release attachments independently:

[attachment]
ENABLED = false

[repository.release.attachment]
PATH = release-attachment

[repository.release.attachment]
PATH = release-attachment

Enable issue / pull request attachments independently:

[repository.release]
ENABLE_ATTACHMENT = false

…t attachment` feature

After a careful analysis of the Gitea attachment feature related code,
I found that initially, the two were simply reused directly.
At least since go-gitea#12465, people have been making them different due to various requirements.
After go-gitea#36697, I think maybe it's a time to completely separate them.

- Add a configuration item [repository.release] -> ENABLE_ATTACHMENT to
  independently control whether to enable release attachments.
- Add a configuration item section [repository.release.attachment] to
  configure an independent storage method for release attachments (optional).

Configuration examples:

Enable both `release attachment` and
`issue / pull request attachment`, configure storage independently.
```INI
[repository.release.attachment]
PATH = release-attachment
```

Enable `release attachments` independently:
```INI
[attachment]
ENABLED = false

[repository.release.attachment]
PATH = release-attachment
```

[repository.release.attachment]
PATH = release-attachment

Enable issue / pull request attachments independently:
```INI
[repository.release.attachment]
ENABLE_ATTACHMENT = false
```

Signed-off-by: a1012112796 <1012112796@qq.com>
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 2, 2026
@github-actions github-actions bot added modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code docs-update-needed The document needs to be updated synchronously labels Apr 2, 2026
Copy link
Copy Markdown
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

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

Can't be right. Don't want to explain more.

@GiteaBot GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 2, 2026
@a1012112796
Copy link
Copy Markdown
Member Author

Can't be right. Don't want to explain more.

@wxiaoguang Before mocking, please fix you stupid bug first.
image

@wxiaoguang
Copy link
Copy Markdown
Contributor

@wxiaoguang Before mocking, please fix you stupid bug first.

Actually, the stupid bug's origin is from your initial PR, because you refused to distinguish different size limits.

And, please open your stupid eye and take a look at the history, it is not written by me.

@a1012112796
Copy link
Copy Markdown
Member Author

@wxiaoguang Before mocking, please fix you stupid bug first.

Actually, the stupid bug's origin is from your initial PR, because you refused to distinguish different size limits.

And, please open your stupid eye and take a look at the history, it is not written by me.

interesting, Since you can mock without giving a reason, why can't I assume it's your problem when I see any bugs?

@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Apr 2, 2026

@wxiaoguang Before mocking, please fix you stupid bug first.

Actually, the stupid bug's origin is from your initial PR, because you refused to distinguish different size limits.
And, please open your stupid eye and take a look at the history, it is not written by me.

interesting, Since you can mock without giving a reason, why can't I assume it's your problem when I see any bugs?

I always give your reasons first (#35812 (comment), the screenshot is clear enough for all information). Maybe you have difficulty to understand others. This one, I didn't , because I have said enough.

@a1012112796
Copy link
Copy Markdown
Member Author

@wxiaoguang Before mocking, please fix you stupid bug first.

Actually, the stupid bug's origin is from your initial PR, because you refused to distinguish different size limits.
And, please open your stupid eye and take a look at the history, it is not written by me.

interesting, Since you can mock without giving a reason, why can't I assume it's your problem when I see any bugs?

I always give your reasons first (#35812 (comment), the screenshot is clear enough for all information). Maybe you have difficulty to understand others. This one, I didn't , because I have said enough.

Thanks for you rely very much. But if I remember correctly, I have replied you for it also. In fact, the code shows that it is still reusing the "Enable" of the attachment, which cannot be changed by this single line of thin comment. so after carfully thinking, I think it's better to fully splite them.

image image

@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Apr 2, 2026

Thanks for you rely very much. But if I remember correctly, I have replied you for it also.

I asked you many times, on different days, where's your reply? 2 days ago (Mar 31, 2026)?

image image

You didn't read the context, didn't read user's bug report, didn't know how the bug is fixed, just say let ME read carefully? Who is the one should read carefully? The legacy storage design&code was indeed written like rubbish, but how it is related to the newly introduced bug and affected users?

image

Did you really understand the problem at that time? What is quota, what is size limit? Does your reply have any single point related to the bug?

image

@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Apr 2, 2026

In fact, the code shows that it is still reusing the "Enable" of the attachment, which cannot be changed by this single line of thin comment.

Indeed, it is still a problem

so after carfully thinking, I think it's better to fully splite them.

I disagree. It shouldn't introduce more config options to make the system unnecessary complex.

Don't pay the cost for unneeded requirements.

I don't see users in moderns still need to disable the attachments. Even if there are such users, the issue/release attachment can be controlled by existing options, for example: set [attachment].MAX_SIZE=0 or MAX_FILES=0 to disable issue attachment while leave release attachment enabled.


In short: keep things simple.

  1. Fix the incorrect UploadAttachmentReleaseSizeLimit call
  2. Update document to clarify that release attachment will be disabled by [attachment].ENABLED
  3. Leave other problems/changes to the future, until there are real users who need them.
;; Whether issue, pull request and release attachments are enabled. Defaults to `true`
;; Release attachment limits are in `[repository.release]` section
;ENABLED = true
;;
;; Comma-separated list for allowed file types for issue and pull request attachments:
;; file extensions (`.zip`), mime types (`text/plain`) or wildcard type (`image/*`, `audio/*`, `video/*`). Empty value or `*/*` allows all types.
;ALLOWED_TYPES = .avif,.cpuprofile,.csv,.dmp,.docx,.fodg,.fodp,.fods,.fodt,.gif,.gz,.jpeg,.jpg,.json,.jsonc,.log,.md,.mov,.mp4,.odf,.odg,.odp,.ods,.odt,.patch,.pdf,.png,.pptx,.svg,.tgz,.txt,.webm,.webp,.xls,.xlsx,.zip
;;
;; Max size of each issue and pull request attachment. Defaults to 100MB
;MAX_SIZE = 100
;;
;; Max number of issue and pull request attachments per upload. Defaults to 5
;MAX_FILES = 5

Another solution is to move ALLOWED_TYPES/MAX_SIZE/MAX_FILES to [repository.issue], but it needs to deprecate old options and it is a breaking change, I don't think we should introduce too many unnecessary breaking changes.

@a1012112796
Copy link
Copy Markdown
Member Author

I asked you many times, on different days, where's your reply? 2 days ago (Mar 31, 2026)?

I'm very sorry, but I'm not an employee of Gitea, so I'm not obliged to reply in a timely manner.

You didn't read the context, didn't read user's bug report, didn't know how the bug is fixed, just say let ME read carefully? Who is the one should read carefully? The legacy storage design&code was indeed written like rubbish, but how it is related to the newly introduced bug and affected users?

Of course, I've read it. Simply put, he doesn't like the current situation where the size limit for release attachments is the same as that for issue attachments. First of all, I don't think this was introduced by my modification strictly speaking; it has been like this before. What I solved was the problem that the back - end of the web UI lacked the implementation of size limits (previously, it didn't actually limit the UI files and purely relied on the front - end for limitation). Secondly, I don't think this is a bug but a feature requirement. The person who designed it initially thought that the attachments for issues and releases were exactly the same thing. From the perspective of simple design, this was completely okay. I have every reason to believe that "github" might have done the same thing at the beginning. However, as people kept using it, they gradually realized that some things should be different. For example, at first, it was found that the link addresses couldn't be the same, and later, it was found that the file types couldn't be the same, and so on. This is just a normal iterative process for an industrial software in actual production. It's not a work of art, and programmers are not gods. The reason why I submitted this new pull request is that after exploration, I recognized that release attachments should be different from issue attachments. But I soon realized that although the comment says "[attachment] -> ENABLED is "whether issue and pull request attachments are enabled. Defaults to true`", in fact, it is still determining whether users can upload release attachments, which obviously doesn't match the comment. Maybe one day someone will think this is a bug and submit an issue. So I chose to completely separate the. May two I ask what the problem is?

I disagree. It shouldn't introduce more config options to make the system unnecessary complex.

So, people are really contradictory. Sometimes they hope for simplicity, and sometimes they hope for flexibility. In fact, GitHub has completely separated them, and they're not even on the same domain anymore.

@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Apr 2, 2026

So, people are really contradictory. Sometimes they hope for simplicity, and sometimes they hope for flexibility.

Clarify: what benefit will this "flexibility" bring? Why it must be done right now, but not deferred to the moment when it is really needed?

image

@a1012112796
Copy link
Copy Markdown
Member Author

So, people are really contradictory. Sometimes they hope for simplicity, and sometimes they hope for flexibility.

Clarify: what benefit will this "flexibility" bring? Why it must be done right now, but not deferred to the moment when it is really needed?

In my view, first , adding a configuration item [repository.release] -> ENABLE_ATTACHMENT not only makes the configuration item [attachment] -> ENABLED consistent with the annotation, but also provides users with the option to independently disable either of them. Secondly, adding [repository.release.attachment] allows users to specify an independent storage method for release attachments. As you said, since there are differences between release attachments and issue attachments in terms of access frequency, size, etc., I have every reason to believe that they may have the need for different storage methods. Of course, I'm open to different views. The reason I directly proposed a PR without creating an issue first is that the modification is very simple, so I presented the proposal directly in code. Thanks.

@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Apr 2, 2026

first ... also provides users with the option to independently disable either of them.

I have said before: are there really such users? Or it is just your imagination?

Secondly .. allows users to specify an independent storage method for release attachments.

Where are such users?

Find such users first, use real world use cases to design and make decision.

@TheFox0x7
Copy link
Copy Markdown
Contributor

Secondly, I don't think this is a bug but a feature requirement

It's a defect in software that made releases unusable for larger uploads without allowing larger issue attachments. No one asked for a new thing but for the previous behavior to work again. So you're correct, it's not a bug. It's a regression though.

I also cannot understand why would you want to disable attachments on releases. What's the usecase here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs-update-needed The document needs to be updated synchronously lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants