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

install_pom.py: interpolate properties in current artifactId, groupId and version + add some tests #98

Merged
merged 6 commits into from
Sep 27, 2024

Conversation

fridrich
Copy link
Contributor

@fridrich fridrich commented Oct 4, 2023

Some artifacts have things like <artifactId>whatever_${scala.binary.version}</artifactId>. The variable normally is defined either in the current pom or in one of its parents. When generating simplified (dependencies-only) pom, interpolate this kind of variables in the current artifact coordinates too.
This would need to be done eventually for mvn_artifact.py too. I tried but did not try a simple elegant way to do it, since the gathering of dependencies happen when the metadata artifact is already intitialized. But eventually one could create a maven artifact from the uart string, interpolate and push the 3 fields back to the metadata artifact. Or create a setter for it in the class.

@codecov-commenter
Copy link

codecov-commenter commented Oct 4, 2023

Codecov Report

Merging #98 (c3818d3) into master (cc0a7e0) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #98      +/-   ##
==========================================
+ Coverage   90.96%   91.00%   +0.03%     
==========================================
  Files          45       45              
  Lines        3397     3400       +3     
==========================================
+ Hits         3090     3094       +4     
+ Misses        307      306       -1     
Files Coverage Δ
java-utils/install_pom.py 95.12% <100.00%> (+0.09%) ⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@fridrich
Copy link
Contributor Author

fridrich commented Oct 4, 2023

Not sure where the codecov misses come from since I added two tests. Especially them being in files I did not touch. But...

@fridrich
Copy link
Contributor Author

fridrich commented Oct 4, 2023

Not sure where the codecov misses come from since I added two tests. Especially them being in files I did not touch. But...

OK, now I understand, it is from my reproducible build changes from some time ago, not from this PR.

@fridrich
Copy link
Contributor Author

Ping?

@fridrich
Copy link
Contributor Author

ping

@fridrich
Copy link
Contributor Author

Ping on this one

@mizdebsk
Copy link
Member

Thanks for the ping, I will review it soon

@@ -172,7 +172,11 @@ def append_if_missing(archive_name, file_name, file_contents):
archive = zipfile.ZipFile(archive_name, 'a')
try:
if file_name not in archive.namelist():
archive.writestr(file_name, file_contents)
file_time = min(4354819199,
max(315532800,
Copy link
Member

Choose a reason for hiding this comment

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

I had to search what these constants are, so a comment would be good.

@mizdebsk
Copy link
Member

Looks good.

@mizdebsk mizdebsk merged commit bbe4995 into fedora-java:master Sep 27, 2024
1 check passed
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.

3 participants