Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions custom/conf/app.example.ini
Original file line number Diff line number Diff line change
Expand Up @@ -1083,6 +1083,9 @@ LEVEL = Info
;; This feature is experimental, not fully tested, and may be changed in the future
;ALLOW_FORK_INTO_SAME_OWNER = false

;; Repository archives (tar.gz, zip and bundle) are streamed when this is enabled instead of being cached
;STREAM_ARCHIVES = true

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;[repository.editor]
Expand Down
2 changes: 2 additions & 0 deletions modules/setting/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ var (
DisableDownloadSourceArchives bool
AllowForkWithoutMaximumLimit bool
AllowForkIntoSameOwner bool
StreamArchives bool

// Repository editor settings
Editor struct {
Expand Down Expand Up @@ -167,6 +168,7 @@ var (
DisableStars: false,
DefaultBranch: "main",
AllowForkWithoutMaximumLimit: true,
StreamArchives: true,

// Repository editor settings
Editor: struct {
Expand Down
3 changes: 3 additions & 0 deletions modules/templates/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,9 @@ func NewFuncMap() template.FuncMap {
"EnableTimetracking": func() bool {
return setting.Service.EnableTimetracking
},
"StreamArchives": func() bool {
return setting.Repository.StreamArchives
},
"DisableWebhooks": func() bool {
return setting.DisableWebhooks
},
Expand Down
31 changes: 22 additions & 9 deletions routers/api/v1/repo/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,28 @@ func archiveDownload(ctx *context.APIContext) {
return
}

// Add nix format link header so tarballs lock correctly:
// https://github.com/nixos/nix/blob/56763ff918eb308db23080e560ed2ea3e00c80a7/doc/manual/src/protocols/tarball-fetcher.md
ctx.Resp.Header().Add("Link", fmt.Sprintf(`<%s/archive/%s.%s?rev=%s>; rel="immutable"`,
ctx.Repo.Repository.APIURL(),
aReq.CommitID,
aReq.Type.String(),
aReq.CommitID,
))

if setting.Repository.StreamArchives {
downloadName := ctx.Repo.Repository.Name + "-" + aReq.GetArchiveName()

ctx.SetServeHeaders(&context.ServeHeaderOptions{
Filename: downloadName,
})

if err := aReq.Stream(ctx, ctx.Repo.GitRepo, ctx.Resp); err != nil && !ctx.Written() {
ctx.APIErrorInternal(err)
}
return
}

archiver, err := aReq.Await(ctx)
if err != nil {
ctx.APIErrorInternal(err)
Expand All @@ -319,15 +341,6 @@ func archiveDownload(ctx *context.APIContext) {
func download(ctx *context.APIContext, archiveName string, archiver *repo_model.RepoArchiver) {
downloadName := ctx.Repo.Repository.Name + "-" + archiveName

// Add nix format link header so tarballs lock correctly:
// https://github.com/nixos/nix/blob/56763ff918eb308db23080e560ed2ea3e00c80a7/doc/manual/src/protocols/tarball-fetcher.md
ctx.Resp.Header().Add("Link", fmt.Sprintf(`<%s/archive/%s.%s?rev=%s>; rel="immutable"`,
ctx.Repo.Repository.APIURL(),
archiver.CommitID,
archiver.Type.String(),
archiver.CommitID,
))

rPath := archiver.RelativePath()
if setting.RepoArchive.Storage.ServeDirect() {
// If we have a signed url (S3, object storage), redirect to this directly.
Expand Down
34 changes: 28 additions & 6 deletions routers/web/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,28 @@ func Download(ctx *context.Context) {
return
}

// Add nix format link header so tarballs lock correctly:
// https://github.com/nixos/nix/blob/56763ff918eb308db23080e560ed2ea3e00c80a7/doc/manual/src/protocols/tarball-fetcher.md
ctx.Resp.Header().Add("Link", fmt.Sprintf(`<%s/archive/%s.%s?rev=%s>; rel="immutable"`,
ctx.Repo.Repository.APIURL(),
aReq.CommitID,
aReq.Type.String(),
aReq.CommitID,
))

if setting.Repository.StreamArchives {
downloadName := ctx.Repo.Repository.Name + "-" + aReq.GetArchiveName()

ctx.SetServeHeaders(&context.ServeHeaderOptions{
Filename: downloadName,
})

if err := aReq.Stream(ctx, ctx.Repo.GitRepo, ctx.Resp); err != nil && !ctx.Written() {
ctx.ServerError("archiver.StreamArchive", err)
}
return
}

archiver, err := aReq.Await(ctx)
if err != nil {
ctx.ServerError("archiver.Await", err)
Expand All @@ -389,12 +411,6 @@ func Download(ctx *context.Context) {
func download(ctx *context.Context, archiveName string, archiver *repo_model.RepoArchiver) {
downloadName := ctx.Repo.Repository.Name + "-" + archiveName

// Add nix format link header so tarballs lock correctly:
// https://github.com/nixos/nix/blob/56763ff918eb308db23080e560ed2ea3e00c80a7/doc/manual/src/protocols/tarball-fetcher.md
ctx.Resp.Header().Add("Link", fmt.Sprintf(`<%s/archive/%s.tar.gz?rev=%s>; rel="immutable"`,
ctx.Repo.Repository.APIURL(),
archiver.CommitID, archiver.CommitID))

rPath := archiver.RelativePath()
if setting.RepoArchive.Storage.ServeDirect() {
// If we have a signed url (S3, object storage), redirect to this directly.
Expand Down Expand Up @@ -423,6 +439,12 @@ func download(ctx *context.Context, archiveName string, archiver *repo_model.Rep
// a request that's already in-progress, but the archiver service will just
// kind of drop it on the floor if this is the case.
func InitiateDownload(ctx *context.Context) {
if setting.Repository.StreamArchives {
ctx.JSON(http.StatusOK, map[string]any{
"complete": true,
})
return
}
aReq, err := archiver_service.NewRequest(ctx.Repo.Repository.ID, ctx.Repo.GitRepo, ctx.PathParam("*"))
if err != nil {
ctx.HTTPError(http.StatusBadRequest, "invalid archive request")
Expand Down
35 changes: 20 additions & 15 deletions services/repository/archiver/archiver.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,25 @@
}
}

// Stream satisfies the ArchiveRequest being passed in. Processing
// will occur directly in this routine.
func (aReq *ArchiveRequest) Stream(ctx context.Context, gitRepo *git.Repository, w io.Writer) error {
if aReq.Type == git.ArchiveBundle {
return gitRepo.CreateBundle(
ctx,
aReq.CommitID,
w,
)
}
return gitRepo.CreateArchive(
ctx,
aReq.Type,
w,
setting.Repository.PrefixArchiveFiles,
aReq.CommitID,
)
}

// doArchive satisfies the ArchiveRequest being passed in. Processing
// will occur in a separate goroutine, as this phase may take a while to
// complete. If the archive already exists, doArchive will not do
Expand Down Expand Up @@ -204,28 +223,14 @@
}
defer gitRepo.Close()

go func(done chan error, w *io.PipeWriter, archiver *repo_model.RepoArchiver, gitRepo *git.Repository) {

Check failure on line 226 in services/repository/archiver/archiver.go

View workflow job for this annotation

GitHub Actions / lint-backend

doArchive$2 - archiver is unused (unparam)

Check failure on line 226 in services/repository/archiver/archiver.go

View workflow job for this annotation

GitHub Actions / lint-go-windows

doArchive$2 - archiver is unused (unparam)

Check failure on line 226 in services/repository/archiver/archiver.go

View workflow job for this annotation

GitHub Actions / lint-go-gogit

doArchive$2 - archiver is unused (unparam)
defer func() {
if r := recover(); r != nil {
done <- fmt.Errorf("%v", r)
}
}()

if archiver.Type == git.ArchiveBundle {
err = gitRepo.CreateBundle(
ctx,
archiver.CommitID,
w,
)
} else {
err = gitRepo.CreateArchive(
ctx,
archiver.Type,
w,
setting.Repository.PrefixArchiveFiles,
archiver.CommitID,
)
}
err = r.Stream(ctx, gitRepo, w)
_ = w.CloseWithError(err)
done <- err
}(done, w, archiver, gitRepo)
Expand Down
1 change: 1 addition & 0 deletions templates/base/head_script.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ If you introduce mistakes in it, Gitea JavaScript code wouldn't run correctly.
pageData: {{.PageData}},
notificationSettings: {{NotificationSettings}}, {{/*a map provided by NewFuncMap in helper.go*/}}
enableTimeTracking: {{EnableTimetracking}},
streamArchives: {{StreamArchives}},
{{if or .Participants .Assignees .MentionableTeams}}
mentionValues: Array.from(new Map([
{{- range .Participants -}}
Expand Down
5 changes: 4 additions & 1 deletion web_src/js/features/repo-common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ async function onDownloadArchive(e: DOMEvent<MouseEvent>) {
}

export function initRepoArchiveLinks() {
queryElems(document, 'a.archive-link[href]', (el) => el.addEventListener('click', onDownloadArchive));
// when archive streaming is enabled, the links will work natively without JS
if (!window.config.streamArchives) {
queryElems(document, 'a.archive-link[href]', (el) => el.addEventListener('click', onDownloadArchive));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

It's too hacky and fragile, there will be bugs in the future if somebody touches the code.

Copy link
Member

@silverwind silverwind Sep 16, 2025

Choose a reason for hiding this comment

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

So what do you suggest? We could alternatively just remove non-streaming mode entirely which would make all the JS and probably a good chunk of Go code deletable (my preference).

Copy link
Contributor

@wxiaoguang wxiaoguang Sep 16, 2025

Choose a reason for hiding this comment

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

Why you need this JS? "GET" request can always start the downloading. So you can just make POST check return immediately for streaming?

Copy link
Member

@silverwind silverwind Sep 16, 2025

Choose a reason for hiding this comment

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

It's still a unneccesary round-trip to the server. It's better to not tamper with these links if we don't need to.

Copy link
Member

@silverwind silverwind Sep 16, 2025

Choose a reason for hiding this comment

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

BTW I think we could eliminate the POST (and the JS) in both cases if we just delay the GET response in non-streaming mode.

Copy link
Contributor

Choose a reason for hiding this comment

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

Any objection?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For me would be your plan ok as well.

On the other hand the current js change makes the file download start sightly faster without right click menu and stops showing me http 400 codes shortly after restarts (invalid csrf tokens? auto fixes itself with later page refresh, GET is not protected)

Copy link
Contributor

@wxiaoguang wxiaoguang Sep 18, 2025

Choose a reason for hiding this comment

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

I will make some changes according to my plan. I think eventually we will get rid of the repo archive and remove a lot of code.


the current js change ....

I would prefer not going with current JS. It couples a lot of modules (source files) together, a single "true/false" value is passed everywhere, which is not ideal. We should avoid such fragile "global variable".

And eventually, we will remove all the legacy JS


showing me http 400 codes shortly after restarts (invalid csrf tokens?

IIRC CSRF token is preserved between restarts, can you reproduce the CSRF token problem by other forms? For example: a comment form?

Copy link
Member

@silverwind silverwind Sep 18, 2025

Choose a reason for hiding this comment

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

Go ahead and revert the JS if you like. I still hope we eventually can change all use cases to GET only, but the POST+GET isn't bothering me too much I guess.

Edit: I see you did already.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you all. Let's keep the unnecessary "POST" for a short time, then completely remove it in next release 🎉

}

export function initRepoActivityTopAuthorsChart() {
Expand Down
1 change: 1 addition & 0 deletions web_src/js/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export type Config = {
mentionValues?: MentionValue[],
mermaidMaxSourceCharacters: number,
i18n: Record<string, string>,
streamArchives: boolean,
};

export type IntervalId = ReturnType<typeof setInterval>;
Expand Down
5 changes: 4 additions & 1 deletion web_src/js/vitest.setup.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import type {Config} from './types.ts';

window.__webpack_public_path__ = '';

window.config = {
Expand All @@ -22,4 +24,5 @@ window.config = {
],
mermaidMaxSourceCharacters: 5000,
i18n: {},
};
streamArchives: true,
} satisfies Config;
Loading