fetchFromGitLab: force re-fetch when rev changes #14

Merged
bbjubjub2494 merged 1 commit from bbjubjub2494/fetchfromgitlab into main 2024-07-01 16:24:14 +00:00
bbjubjub2494 commented 2024-05-21 08:57:00 +00:00 (Migrated from github.com)

There are a few instances of fetchFromGitLab in the repo, for instance gtk-doc.src.

My primary goal when writing this was changing the output derivation following #11. This is doable with a lot less diff in fetchFromGitLab, but it's worth asking whether both fetchers should be different and why. I think a lot of the divergence comes from years of random commits rather than from design.

There are a few instances of `fetchFromGitLab` in the repo, for instance `gtk-doc.src`. My primary goal when writing this was changing the output derivation following #11. This is doable with a lot less diff in `fetchFromGitLab`, but it's worth asking whether both fetchers should be different and why. I think a lot of the divergence comes from years of random commits rather than from design.
Sorixelle commented 2024-05-22 12:51:33 +00:00 (Migrated from github.com)

I'd be inclined to rework both, and use what exists now as a base for a generic fetchFromGitForge that both fetchFromGitHub and fetchFromGitlab can be defined in terms of.

I'd be inclined to rework both, and use what exists now as a base for a generic `fetchFromGitForge` that both `fetchFromGitHub` and `fetchFromGitlab` can be defined in terms of.
bbjubjub2494 commented 2024-06-04 20:37:11 +00:00 (Migrated from github.com)

I tried working on the refactor in a branch, but I am not comfortable writing such code for production because there seem to be too many small details in the original code like complex passthroughs and unsafe functions that I don't know the purpose of.
https://github.com/bbjubjub2494/auxolotl_core/tree/fetchfromgitforge

I would suggest we do the minimal diff thing for now

I tried working on the refactor in a branch, but I am not comfortable writing such code for production because there seem to be too many small details in the original code like complex passthroughs and unsafe functions that I don't know the purpose of. https://github.com/bbjubjub2494/auxolotl_core/tree/fetchfromgitforge I would suggest we do the minimal diff thing for now
Owner

I think we should be good to merge this. This shouldn't cause a rebuild since the source is identical right.

I think we should be good to merge this. This shouldn't cause a rebuild since the source is identical right.
Member

I think we should be good to merge this. This shouldn't cause a rebuild since the source is identical right.

Just tested with busybox, it looks like it does cause a rebuild

> I think we should be good to merge this. This shouldn't cause a rebuild since the source is identical right. Just tested with busybox, it looks like it does cause a rebuild
Owner

The more I think about it the less important the rebuild is since we are providing our own package versions anyways. So I think we can just ignore rebuilds for the time being.

The more I think about it the less important the rebuild is since we are providing our own package versions anyways. So I think we can just ignore rebuilds for the time being.
isabelroses merged commit 8be7956fc0 into main 2024-07-01 16:24:14 +00:00
Member

Doesn't seem to have caused a rebuild on hydra, I don't know it should have! (I've only got jobs for stdenv and lix set-up so far.)

Doesn't seem to have caused a rebuild on hydra, I don't know it should have! (I've only got jobs for stdenv and lix set-up so far.)
Owner

Doesn't seem to have caused a rebuild on hydra, I don't know it should have! (I've only got jobs for stdenv and lix set-up so far.)

I believe this PR was only for gitlab sources, perhaps make a gitlab test?

> Doesn't seem to have caused a rebuild on hydra, I don't know it should have! (I've only got jobs for stdenv and lix set-up so far.) I believe this PR was only for gitlab sources, perhaps make a gitlab test?
Member

Ah, I wasn't attempting to test so much as provide feedback on whether it caused a rebuild!

Ah, I wasn't attempting to test so much as provide feedback on whether it caused a rebuild!
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
4 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: auxolotl/core#14
No description provided.