Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix: executerbacgen even if dependencies' mod times are older than source file's#13757

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
dannykopping merged 2 commits intomainfromdk/rbac-make
Jul 2, 2024

Conversation

dannykopping
Copy link
Contributor

I'm using GNU Make 4.4.1

When executingcodersdk/rbacresources_gen.go, I found that the make target was not actually being executed. I traced this down to the dependencies' modification times being less than the target file's. This appears to be a performance-related behaviour inmake, since a target file would not need to change if all of its dependencies were modified before it.

In this case, however, we need to execute thescripts/rbacgen/main.go script even if it has not been recently modified, and the solution to this is to add aPHONY target.

I suspect we have other cases in ourMakefile which are preventing targets from running when we expect them to.

# reset mtimes for dependencies to a date in the past$ touch -t202405020000 scripts/rbacgen/codersdk.gotmpl scripts/rbacgen/main.go coderd/rbac/object.go# without PHONY, no change$ make codersdk/rbacresources_gen.go$ git diff --stat -- codersdk/rbacresources_gen.go| cat# no changes# set all mtimes to now$ touch scripts/rbacgen/codersdk.gotmpl scripts/rbacgen/main.go coderd/rbac/object.go# without PHONY, target executes$ make codersdk/rbacresources_gen.go$ git diff --stat -- codersdk/rbacresources_gen.go| cat codersdk/rbacresources_gen.go| 102 +++++++++++++++++++++--------------------- 1 file changed, 52 insertions(+), 50 deletions(-)-----------------------------------------------------------------------------------------------------------# reset$ git checkout codersdk/rbacresources_gen.go# reset mtimes for dependencies to a date in the past$ touch -t202405020000 scripts/rbacgen/codersdk.gotmpl scripts/rbacgen/main.go coderd/rbac/object.go# with PHONY$ git diff --stat -- codersdk/rbacresources_gen.go| cat codersdk/rbacresources_gen.go| 102 +++++++++++++++++++++--------------------- 1 file changed, 52 insertions(+), 50 deletions(-)

… fileSigned-off-by: Danny Kopping <danny@coder.com>
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Are we missingcodersdk/rbacresources_gen.go ingen/mark-fresh for a reason?

Copy link
Member

Choose a reason for hiding this comment

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

Could just be an omission?

Copy link
Member

@EmyrkEmyrkJul 2, 2024
edited
Loading

Choose a reason for hiding this comment

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

It's an omission. There is no reason. We should add it

dannykopping reacted with thumbs up emoji
Copy link
Member

@EmyrkEmyrk left a comment

Choose a reason for hiding this comment

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

PHONY target is fine, but can we just add thepolicy.go file as a dependency? Then updates to that would work?

But adding PHONY so it runs on every gen seems fine to me too. I usually run with-B.

Signed-off-by: Danny Kopping <danny@coder.com>
@dannykopping
Copy link
ContributorAuthor

@Emyrk yeah, on reflection addingpolicy.go as a dependency is more idiomatic.
I worry that the same problem will come up later if we include other files in the code gen, but for now this seems like the better approach.
I've added thegen/mark-fresh line as well.

Emyrk reacted with thumbs up emoji

@dannykoppingdannykoppingenabled auto-merge (squash)July 2, 2024 14:23
@dannykoppingdannykopping merged commit98c09bf intomainJul 2, 2024
29 of 31 checks passed
@dannykoppingdannykopping deleted the dk/rbac-make branchJuly 2, 2024 14:29
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsJul 2, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@johnstcnjohnstcnjohnstcn approved these changes

@EmyrkEmyrkEmyrk approved these changes

Assignees

@dannykoppingdannykopping

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@dannykopping@johnstcn@Emyrk

[8]ページ先頭

©2009-2025 Movatter.jp