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

Import local schema to avoid DNS lookup in build #7283

Merged
merged 1 commit into from
Apr 23, 2024

Conversation

jgneff
Copy link
Contributor

@jgneff jgneff commented Apr 19, 2024

Please review this one-line change to the file jakartaee_11.xsd, which is a new file for NetBeans 22. This change in the Jakarta EE 11 file is the same as the previous changes for Jakarta EE 10 in 2023 and Jakarta EE 9 in 2022:

The original issue is:

Without this fix, building NetBeans on Launchpad fails with the exception:

/build/snapcraft-strictly-netbeans-79a4131177e2fda97a9ea091531b589f/parts/...
    netbeans/build/build-release-temp/enterprise/j2ee.dd/build.xml:219:
    Failed to parse document at 'https://www.w3.org/2001/xml.xsd'.
java.net.UnknownHostException: www.w3.org
    at java.base/sun.nio.ch.NioSocketImpl.connect(NioSocketImpl.java:567)
    at java.base/java.net.SocksSocketImpl.connect(SocksSocketImpl.java:327)
    at java.base/java.net.Socket.connect(Socket.java:633)

With this fix, there are no DNS lookups during the build, and the NetBeans build on Launchpad is successful.

@jgneff
Copy link
Contributor Author

jgneff commented Apr 19, 2024

Did I do this right? I created my pull request branch import-local-schema from the delivery branch, but it seems I forgot to switch the target branch name on GitHub when submitting the pull request. Is there a way to simply move this over to the delivery branch, or do I need to close this and do it over? Thanks!

UPDATE: Never mind. I fixed it after finding these helpful instructions.

@jgneff jgneff changed the base branch from master to delivery April 19, 2024 23:23
@mbien
Copy link
Member

mbien commented Apr 20, 2024

hi @jgneff,

Did I do this right?

Depends what you are trying to do ;) master would target NB 23 right now, delivery is essentially bugfix/lowrisk-only and targets NB 22 which is in the release candidate phase.

Regarding changing the target branch of a PR: your branch should be a copy of the branch you are targeting. So if you target delivery, make sure your commit is on top of a copy of the delivery branch. Same for master etc.

You don't have to close the PR, you can simply force push into your branch (-f), github will catch up.

@jgneff
Copy link
Contributor Author

jgneff commented Apr 20, 2024

Thanks, Michael. I would like this to go into NetBeans 22, if at all possible, but I'm not able to add any labels. The build of NetBeans 22 fails for me without this fix (as it did for the same problem in 2022 and 2023).

Depends what you are trying to do ;) master would target NB 23 right now, delivery is essentially bugfix/lowrisk-only and targets NB 22 which is in the release candidate phase.

I figured it out. (See UPDATE above.) I didn't realize you could edit the base branch in addition to the PR title when you click the Edit button at the top of the pull request on GitHub.

Regarding changing the target branch of a PR: your branch should be a copy of the branch you are targeting. So if you target delivery, make sure your commit is on top of a copy of the delivery branch.

I did that. I just forgot to change the base branch on the GitHub pull request. GitHub sets the base branch to the master branch no matter what you have for the actual base of your pull request branch, it seems.

You don't have to close the PR, you can simply force push into your branch (-f), github will catch up.

Didn't even have to do that. I just switched it in GitHub to match what I had actually done locally, which is to base my pull request branch on the delivery branch.

@mbien mbien added enterprise [ci] enable enterprise job build labels Apr 20, 2024
@mbien
Copy link
Member

mbien commented Apr 20, 2024

sounds good. going to quickly lock and unlock this PR so that CI is picking up the labels. The file is also recent, so it makes sense that it started happening not too long ago.

edit: all tests are green, removing label

@apache apache locked and limited conversation to collaborators Apr 20, 2024
@apache apache unlocked this conversation Apr 20, 2024
@mbien mbien added this to the NB22 milestone Apr 20, 2024
@mbien mbien added the ci:all-tests [ci] enable all tests label Apr 20, 2024
@apache apache locked and limited conversation to collaborators Apr 20, 2024
@apache apache unlocked this conversation Apr 20, 2024
@mbien mbien removed the ci:all-tests [ci] enable all tests label Apr 20, 2024
Copy link
Contributor

@matthiasblaesing matthiasblaesing left a comment

Choose a reason for hiding this comment

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

In general looks good and can go in. Thank you.

@jgneff do you have the full stack trace when this happens? Ideally we would not need to fiddle with the schemas and just use a catalog to modify the resolving, just as the IDE normally does.

@jgneff
Copy link
Contributor Author

jgneff commented Apr 20, 2024

@jgneff do you have the full stack trace when this happens? Ideally we would not need to fiddle with the schemas and just use a catalog to modify the resolving, just as the IDE normally does.

Here's the full stack trace: strictly-netbeans_amd64.txt. This pull request simply loads the copy of the latest 2009 XML namespace schema that is already found on the local file system:

enterprise/j2ee.dd/src/org/netbeans/modules/j2ee/dd/impl/resources/jakartaee_11.xsd now refers to:
enterprise/j2ee.dd/src/org/netbeans/modules/j2ee/dd/impl/resources/xml.xsd

@ebarboni ebarboni merged commit 5094b19 into apache:delivery Apr 23, 2024
42 of 70 checks passed
@jgneff jgneff deleted the import-local-schema branch April 23, 2024 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build enterprise [ci] enable enterprise job
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants