Skip to content

remove timestamp from generated files #350

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.

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

Merged
merged 1 commit into from
Sep 26, 2023
Merged

remove timestamp from generated files #350

merged 1 commit into from
Sep 26, 2023

Conversation

katexochen
Copy link
Contributor

This enables reproducible OS builds.
Fixes #319

This is an alternative approach to #348, based on the suggestion by @cgwalters

@travier
Copy link
Contributor

travier commented Sep 6, 2023

+1 from me for this PR as well. (not a maintainer either but an interested party like Colin).

@pbrezina
Copy link
Member

Hi, so I finally have some time to put into authselect. Thank you for the contribution.

Do I understand it correctly that there are two distinct solutions and that we merge only one? That is:

@katexochen
Copy link
Contributor Author

Do I understand it correctly that there are two distinct solutions and that we merge only one? That is:

@pbrezina exactly, and I'd say that this PR here is the preferred solution. :)

@pbrezina
Copy link
Member

Yes, I agree. I don't think there is any particular reason to keep the timestamp in the preamble.

I would just like to keep template_generate_preamble and only remove the timestamp parameter.

@pbrezina
Copy link
Member

Also is Fedora rawhide enough or do you want to deliver this also to F39 or older?

@travier
Copy link
Contributor

travier commented Sep 22, 2023

F39 would be nice

@katexochen
Copy link
Contributor Author

I would just like to keep template_generate_preamble and only remove the timestamp parameter.

Done.

@katexochen
Copy link
Contributor Author

Also is Fedora rawhide enough or do you want to deliver this also to F39 or older?

F38 would be great.

@pbrezina
Copy link
Member

Please, apply this diff and we can merge:

diff --git a/src/lib/util/template.c b/src/lib/util/template.c
index 98d762e..f211d86 100644
--- a/src/lib/util/template.c
+++ b/src/lib/util/template.c
@@ -555,7 +555,8 @@ template_generate_preamble()
         "# Do not modify this file manually, use authselect instead. Any user changes will be overwritten.\n"
         "# You can stop authselect from managing your configuration by calling 'authselect opt-out'.\n"
         "# See authselect(8) for more details.\n\n";
-    return preamble;
+
+    return strdup(preamble);
 }
 
 errno_t
@@ -668,15 +669,18 @@ template_write(const char *filepath,
                const char *content,
                mode_t mode)
 {
-    const char *preamble = template_generate_preamble();
+    char *preamble;
     char *output;
     errno_t ret;
 
+    preamble = template_generate_preamble();
     if (content == NULL) {
-        output = strdup(preamble);
+        output = preamble;
     } else {
         output = format("%s%s", preamble, content);
+        free(preamble);
     }
+
     if (output == NULL) {
         return ENOMEM;
     }

Also the commit message should look like this:

    templates: remove timestamp from generated files

    This enables reproducible OS builds.

    Resolves: https://github.com/authselect/authselect/issues/319

    Co-authored-by: Malte Poll <[email protected]>
    Signed-off-by: Paul Meyer <[email protected]>

Thank you.

This enables reproducible OS builds.

Resolves: #319

Co-authored-by: Malte Poll <[email protected]>
Signed-off-by: Paul Meyer <[email protected]>
@pbrezina
Copy link
Member

Thank you.

@pbrezina pbrezina merged commit 44b9d87 into authselect:master Sep 26, 2023
@katexochen katexochen deleted the remove-timestamps-2 branch September 26, 2023 13:01
@pbrezina
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support SOURCE_DATE_EPOCH for artifacts
4 participants