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.

Sign up for GitHub

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

Jump to bottom

Ant task to automatically convert to a simple shim layout #7561

Merged
merged 128 commits into from
Feb 23, 2023
Merged

Ant task to automatically convert to a simple shim layout #7561

merged 128 commits into from
Feb 23, 2023

Conversation

gerashegalov
Copy link
Collaborator

@gerashegalov gerashegalov commented Jan 20, 2023 •
edited
Loading

Closes #7528

Usage:

mvn -B clean generate-sources antrun:run@shimplify-shim-sources -pl sql-plugin -Dshimplify=true -Dshimplify.add.base=323 -Dshimplify.add.shim=324
  • Prints warning if detects a shim containing multiple source roots not shared with other shims. By construction a shim should have only a single directory that is not shared. Example:
shimplify - WARNING - Consider consolidating 340, it spans multiple dedicated directories ['/home/user/gits/NVIDIA/spark-rapids/sql-plugin/src/main/340+/scala', '/home/user/gits/NVIDIA/spark-rapids/sql-plugin/src/main/340/scala']
shimplify - WARNING - Consider consolidating 312db, it spans multiple dedicated directories ['/home/user/gits/NVIDIA/spark-rapids/sql-plugin/src/main/312db/scala', '/home/user/gits/NVIDIA/spark-rapids/sql-plugin/src/main/31xdb/scala']
  • Updates shimplfy json comments without moving the files they are the old locations
  • Move files to the canonical fist-shim-affected location
  • Clones an existing shim to a new shim
  • Removes an existing shim

The goal is to deprecate the range dirs.

Signed-off-by: Gera Shegalov [email protected]

@gerashegalov
wip
60f53e1
@gerashegalov
311 compiles
ec1da64 
Signed-off-by: Gera Shegalov <[email protected]>
@gerashegalov
31X compiles
bce0644 
Signed-off-by: Gera Shegalov <[email protected]>
@gerashegalov
320 compiles
acdc350 
Signed-off-by: Gera Shegalov <[email protected]>
@gerashegalov
321 compiles
23c1870 
Signed-off-by: Gera Shegalov <[email protected]>
@gerashegalov
321cdh compiles
e56934f 
Signed-off-by: Gera Shegalov <[email protected]>
@gerashegalov
321db TBD
72c1a1f 
Signed-off-by: Gera Shegalov <[email protected]>
@gerashegalov
322 compiles
b07753c 
Signed-off-by: Gera Shegalov <[email protected]>
@gerashegalov
323 compiles
e7f81c7 
Signed-off-by: Gera Shegalov <[email protected]>
@gerashegalov
330 compiles
2db8372 
Signed-off-by: Gera Shegalov <[email protected]>
@gerashegalov
330cdh compiles
9224daa 
Signed-off-by: Gera Shegalov <[email protected]>
@gerashegalov
331 compiles
364c799 
Signed-off-by: Gera Shegalov <[email protected]>
@gerashegalov
340 compiles
dfb28fb 
Signed-off-by: Gera Shegalov <[email protected]>
@gerashegalov
312db compiles
acf1a1d 
Signed-off-by: Gera Shegalov <[email protected]>
@gerashegalov
Merge remote-tracking branch 'origin/branch-23.02' into generateSymlinks
9f23a64
@gerashegalov
Markdown update
3895d8a 
Signed-off-by: Gera Shegalov <[email protected]>
@gerashegalov
332 compiles
388a778 
Signed-off-by: Gera Shegalov <[email protected]>
@gerashegalov
symlinks optin
675dbc7
@gerashegalov
gen
34b7362 
Signed-off-by: Gera Shegalov <[email protected]>
@gerashegalov
typo
8f04d3e 
Signed-off-by: Gera Shegalov <[email protected]>
@gerashegalov
new tests
c4069fa
@gerashegalov
move to parent pom
af6d194
@gerashegalov
Unify with tests
1e80aef 
Signed-off-by: Gera Shegalov <[email protected]>
@gerashegalov
scalastyle
d0a0bab
@gerashegalov
add tests to validate
1b7b953
@gerashegalov
scalastyle
33a17a3 
Signed-off-by: Gera Shegalov <[email protected]>
@gerashegalov
remove extra newline
d9b3ba7 
Signed-off-by: Gera Shegalov <[email protected]>
@gerashegalov
Merge remote-tracking branch 'origin/branch-23.02' into generateSymlinks
4197e2a
@gerashegalov
shim list validation
5db6c19 
Signed-off-by: Gera Shegalov <[email protected]>
@gerashegalov
JSON format and validation prototype
4a8c556 
Signed-off-by: Gera Shegalov <[email protected]>
razajafri
razajafri reviewed Feb 16, 2023
View reviewed changes
build/shimplify.py Show resolved Hide resolved
docs/dev/shimplify.md Outdated Show resolved Hide resolved
docs/dev/shimplify.md Show resolved Hide resolved
docs/dev/shimplify.md Outdated Show resolved Hide resolved
docs/dev/shimplify.md Show resolved Hide resolved
docs/dev/shimplify.md Outdated Show resolved Hide resolved
docs/dev/shimplify.md Show resolved Hide resolved
docs/dev/shimplify.md Outdated Show resolved Hide resolved
razajafri
razajafri reviewed Feb 16, 2023
View reviewed changes
build/shimplify.py Outdated Show resolved Hide resolved
@razajafri
Copy link
Collaborator

Great work! I have left a few comments but I still haven't gone over all the implementation details

@razajafri
Copy link
Collaborator

We should also have a follow-on issue for running this and merging the new structure

gerashegalov and others added 2 commits February 16, 2023 23:56
@gerashegalov
Copy link
Collaborator Author

We should also have a follow-on issue for running this and merging the new structure

added a task in #7487 . I have a pending PR that whose diff will be replaced as generated by shimplify

@gerashegalov
Copy link
Collaborator Author

build

razajafri
razajafri previously approved these changes Feb 17, 2023
View reviewed changes
@gerashegalov
bug fixes
0b18c7c 
-  __add_shim_buildver, __remove_shim_buildver imply should_overwrite
- don't git rm files unless shimplify.move=true

Signed-off-by: Gera Shegalov <[email protected]>
jlowe
jlowe previously approved these changes Feb 21, 2023
View reviewed changes
Copy link
Member

@jlowe jlowe left a comment

Choose a reason for hiding this comment

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

Minor typo, lgtm.

build/shimplify.py Outdated Show resolved Hide resolved
razajafri
razajafri previously approved these changes Feb 21, 2023
View reviewed changes
@gerashegalov gerashegalov dismissed stale reviews from razajafri and jlowe via 51100db February 21, 2023 20:56
@gerashegalov
Copy link
Collaborator Author

build

nartal1
nartal1 approved these changes Feb 22, 2023
View reviewed changes
Copy link
Collaborator

@nartal1 nartal1 left a comment

Choose a reason for hiding this comment

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

Great work! I tried these instructions locally and it would be much easier to remove/add shims with this approach.

@gerashegalov
Copy link
Collaborator Author

build

@gerashegalov gerashegalov merged commit c5b8016 into NVIDIA:branch-23.04 Feb 23, 2023
@gerashegalov gerashegalov deleted the generateSymlinks-just-build-v1 branch February 23, 2023 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nartal1 nartal1 nartal1 approved these changes

@jlowe jlowe jlowe approved these changes

@revans2 revans2 Awaiting requested review from revans2 revans2 is a code owner

@GaryShen2008 GaryShen2008 Awaiting requested review from GaryShen2008 GaryShen2008 is a code owner

@NvTimLiu NvTimLiu Awaiting requested review from NvTimLiu NvTimLiu is a code owner

@zhanga5 zhanga5 Awaiting requested review from zhanga5

@tgravescs tgravescs Awaiting requested review from tgravescs tgravescs is a code owner

@razajafri razajafri Awaiting requested review from razajafri

Assignees

@gerashegalov gerashegalov

Labels
documentation Improvements or additions to documentation task Work required that improves the product but is not user facing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Maven goal to convert an old-style shim to a shimple shim
6 participants
@gerashegalov @razajafri @jlowe @tgravescs @nartal1 @sameerz