Skip to content
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

Add support to embed source metadata from git #1444

Merged
merged 9 commits into from
Aug 11, 2022

Conversation

imnitishng
Copy link
Contributor

@imnitishng imnitishng commented May 17, 2022

Summary

Add support to add source metadata from git, to complete #1280

  • Support current branch refs
  • Support current tag refs
  • Support git describe --tags --always
  • Support git remote
  • Add unit tests
  • Add acceptance tests
  • Clean and organize code with proper error handling

Output

Before

After

Documentation

  • Should this change be documented?
    • Yes, see #___
    • No

Related

Resolves #1137

@imnitishng imnitishng requested a review from a team as a code owner May 17, 2022 16:26
@github-actions github-actions bot added the type/enhancement Issue that requests a new feature or improvement. label May 17, 2022
@github-actions github-actions bot added this to the 0.27.0 milestone May 17, 2022
@imnitishng imnitishng marked this pull request as draft May 17, 2022 16:27
@imnitishng imnitishng changed the title Add support to add source metadata from git Add support to embed source metadata from git May 17, 2022
@github-actions github-actions bot added the type/chore Issue that requests non-user facing changes. label May 17, 2022
Copy link
Member

@dfreilich dfreilich left a comment

Choose a reason for hiding this comment

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

This looks like a really great start!

Our pkg/client directory is already pretty full, and we seem to typically put our direct commands there. I would instead suggest putting the metadata.go file (and accompanying test) in the pkg/project/v2 directory). What do you think, @jromero ?

pkg/client/metadata.go Outdated Show resolved Hide resolved
pkg/client/metadata.go Outdated Show resolved Hide resolved
pkg/client/metadata.go Outdated Show resolved Hide resolved
pkg/client/metadata.go Outdated Show resolved Hide resolved
@imnitishng
Copy link
Contributor Author

imnitishng commented May 22, 2022

I would instead suggest putting the metadata.go file (and accompanying test) in the pkg/project/v2 directory). What do you think, @jromero ?

Once this is confirmed by others, I'll move the files
Working on fixing failing tests for now.


There is a couple of things on git vs our implementation of the commands that I wanted to point out here

  • git describe --always returns the short hash of a commit, whose length could vary based on the number of commits (to be unique) in the repository. So for now I went with returning the full commit hash.
    • In the same way full hash is used to generate the tag in the case if the tag exists in a different branch from HEAD, more details on how this tag is generated by git. So git would output v1.0.4-14-g2414721 but this implementation would output v1.0.4-14-g2414721234l23wer323sas58sd8k.
      • The prefix -g here denotes that the tag was made using git SCM, since we are only dealing with git repositories here for now, could we omit this?
  • In case of multiple tags git describe --tags --always priortizes annotated tags over unannotated and if there are multiple tags of the same type then they are sorted based on creation date for annotated tags and alphabetical order for unannotated tags, this behaviour was observed by me during testing, hence I have implemented it in the logic.

I am not too sure how to write acceptance tests for this, (are they even required or not?) but if we decide on writing it then we'll probably have to create some dummy files and folders that act as our git repository to test on. Let me know if there is another approach available.

Signed-off-by: Nitish Gupta <imnitish.ng@gmail.com>
Signed-off-by: Nitish Gupta <imnitish.ng@gmail.com>
Signed-off-by: Nitish Gupta <imnitish.ng@gmail.com>
- Add support to fetch and save the origin remote URL of a git repo

Signed-off-by: Nitish Gupta <imnitish.ng@gmail.com>
Signed-off-by: Nitish Gupta <imnitish.ng@gmail.com>
- Add ability to parse tags if they exist in multiple branches
- Add test cases to cover possible branching and tag scenarios
- Add new util method and tests

Signed-off-by: Nitish Gupta <imnitish.ng@gmail.com>
@imnitishng imnitishng force-pushed the 13-05-git-source-metadata branch 5 times, most recently from 54a5a27 to 713984d Compare May 23, 2022 12:20
@codecov
Copy link

codecov bot commented May 23, 2022

Codecov Report

Merging #1444 (58c1f56) into main (da32cb8) will decrease coverage by 0.10%.
The diff coverage is 87.60%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1444      +/-   ##
==========================================
- Coverage   81.38%   81.29%   -0.09%     
==========================================
  Files         152      154       +2     
  Lines        9864    10014     +150     
==========================================
+ Hits         8027     8140     +113     
- Misses       1360     1393      +33     
- Partials      477      481       +4     
Flag Coverage Δ
os_linux 80.01% <87.60%> (-0.13%) ⬇️
os_macos 77.48% <87.60%> (-0.06%) ⬇️
os_windows 81.17% <87.60%> (-0.08%) ⬇️
unit 81.29% <87.60%> (-0.09%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@imnitishng
Copy link
Contributor Author

I might also need some pointers on the failing test cases, not sure what is happening and it's also taking a long time for it to complete.

@imnitishng imnitishng marked this pull request as ready for review May 23, 2022 12:48
Signed-off-by: Nitish Gupta <imnitish.ng@gmail.com>
@imnitishng imnitishng requested a review from dfreilich May 25, 2022 18:31
@jromero jromero modified the milestones: 0.27.0, 0.28.0 Jun 3, 2022
@imnitishng imnitishng self-assigned this Jun 28, 2022
@jromero jromero self-assigned this Aug 2, 2022
Signed-off-by: Javier Romero <root@jromero.codes>
Signed-off-by: Javier Romero <root@jromero.codes>
@jromero jromero removed the type/chore Issue that requests non-user facing changes. label Aug 5, 2022
@jromero
Copy link
Member

jromero commented Aug 11, 2022

I am not too sure how to write acceptance tests for this, (are they even required or not?) but if we decide on writing it then we'll probably have to create some dummy files and folders that act as our git repository to test on. Let me know if there is another approach available.

I'm going to merge this as is given the code coverage at the unit level. I'm working on refactoring the acceptance test suite to be easier to work with. As part of that work, I'll see if we can test this set of functionality as well.

@jromero jromero merged commit 6575218 into buildpacks:main Aug 11, 2022
@jromero jromero mentioned this pull request Aug 11, 2022
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement Issue that requests a new feature or improvement.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Provide project source metadata from local git information
3 participants